licenses_cpp: Switched to lexically_relative for 2x speed boost. #173048
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request improves performance by replacing std::filesystem::relative with std::filesystem::path::lexically_relative. The changes look good and should provide a speed boost. There's one potential correctness issue in FileRun where a path is not normalized before being used, which could lead to incorrect behavior with lexically_relative. I've left a suggestion to fix it.
Contributor
There was a problem hiding this comment.
Code Review
This pull request aims to improve performance by switching from fs::relative to fs::lexically_relative. The changes look good and correctly make paths absolute before comparison. However, I've found a potential correctness issue where paths are not normalized before being used with lexically_relative, which could lead to incorrect behavior with certain path formats. I've suggested fixes for this to ensure correctness.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
gaaclarke
changed the title
jtmcdole
approved these changes
Jul 31, 2025
Member
Author
|
Here's the run for it, 21s on the linux bot: https://ci.chromium.org/ui/p/flutter/builders/try/Mac%20Engine%20Drone/1850102/overview Those linux virtual machines were very sensitive to disk access. I was getting 2 minutes sometimes on mine. |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 1, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 1, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 1, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 1, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 1, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 1, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 1, 2025
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Aug 1, 2025
Roll Flutter from c3279caa127d to 871849e4b6bf (56 revisions) flutter/flutter@c3279ca...871849e 2025-08-01 [email protected] Roll Dart SDK from 6832e04cf2f9 to 6e1bb159c860 (8 revisions) (flutter/flutter#173119) 2025-08-01 [email protected] Add `--profile-startup` flag to `flutter run` (flutter/flutter#172990) 2025-08-01 [email protected] Add `side` to `RadioThemeData` (flutter/flutter#171945) 2025-08-01 [email protected] Update GCA instructions (flutter/flutter#173001) 2025-08-01 [email protected] [engine] Null aware elements clean-ups (flutter/flutter#173075) 2025-08-01 [email protected] Roll Packages from db6988d to f0645d8 (3 revisions) (flutter/flutter#173111) 2025-08-01 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland licenses cpp switch 3 (#173063)" (flutter/flutter#173113) 2025-08-01 [email protected] Update embedder API CODEOWNERS (flutter/flutter#173081) 2025-08-01 [email protected] Move android_obfuscate_test from devicelab into tools integration.shard (flutter/flutter#169798) 2025-08-01 [email protected] [A11y] RangeSlider should have 2 focus node (flutter/flutter#172729) 2025-07-31 [email protected] Upload the linux arm64 embedder to cloud buckets. (flutter/flutter#173068) 2025-07-31 [email protected] Reland licenses cpp switch 3 (flutter/flutter#173063) 2025-07-31 [email protected] [ Tool ] Mark IOOverrides subclasses as `final` (flutter/flutter#173078) 2025-07-31 [email protected] [macOS] Remove duplicate object initialization (flutter/flutter#171767) 2025-07-31 [email protected] Redistribute Android test owners (flutter/flutter#172886) 2025-07-31 [email protected] Avoid negatives in the styleguide.md (flutter/flutter#172917) 2025-07-31 [email protected] Roll Skia from 42e3bed42110 to 49e39cd3cf18 (7 revisions) (flutter/flutter#173069) 2025-07-31 [email protected] Fix the issue where calling showOnScreen on a sliver after a pinned child in SliverMainAxisGroup does not reveal it. (flutter/flutter#171339) 2025-07-31 [email protected] Improve assertion message in `EdgeInsetsDirectional.resolve` (flutter/flutter#172099) 2025-07-31 [email protected] licenses_cpp: Switched to lexically_relative for 2x speed boost. (flutter/flutter#173048) 2025-07-31 [email protected] Add dartvm to the dart_sdk_entitlement_config list. (flutter/flutter#173044) 2025-07-31 [email protected] [web] ClickDebouncer workaround for iOS Safari click behavior (flutter/flutter#172995) 2025-07-31 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland licenses cpp switch 2 (#172996)" (flutter/flutter#173059) 2025-07-31 [email protected] [web] Text editing test accepts both behaviors in Firefox (flutter/flutter#172767) 2025-07-31 [email protected] Reland licenses cpp switch 2 (flutter/flutter#172996) 2025-07-31 [email protected] Roll Packages from d914120 to db6988d (2 revisions) (flutter/flutter#173039) 2025-07-31 [email protected] Add a SliverList code sample (flutter/flutter#172986) 2025-07-31 [email protected] fix: adjust scrollable size assertion with tolerance (flutter/flutter#171426) 2025-07-31 [email protected] impeller: Shrink `Command` 40 bytes (flutter/flutter#173004) 2025-07-31 [email protected] [web] Remove outdated comment about HTML renderer (flutter/flutter#172877) 2025-07-31 [email protected] Roll Skia from ff4da49305c5 to 42e3bed42110 (1 revision) (flutter/flutter#173038) 2025-07-31 [email protected] Roll Skia from 7d468a4868cb to ff4da49305c5 (3 revisions) (flutter/flutter#173028) 2025-07-31 [email protected] Roll Skia from 951277895c86 to 7d468a4868cb (1 revision) (flutter/flutter#173027) 2025-07-31 [email protected] Manual roll of Dart from 14ea8d342149 to 6832e04cf2f9 (flutter/flutter#173015) 2025-07-31 [email protected] Roll Skia from 8bdf4cdcf4ed to 951277895c86 (2 revisions) (flutter/flutter#173022) 2025-07-31 [email protected] Roll Fuchsia Linux SDK from bQVQlLssTxxLjoDU0... to xo_bqOoFuBKFmgKxn... (flutter/flutter#173021) 2025-07-31 [email protected] feat: Add `cursorHeight` to `DropdownMenu` (flutter/flutter#172615) 2025-07-31 [email protected] Roll Skia from a3347cee2d73 to 8bdf4cdcf4ed (3 revisions) (flutter/flutter#173013) 2025-07-31 [email protected] [ Widget Preview ] Add `--web-server` support (flutter/flutter#172978) 2025-07-30 [email protected] Bump customer tests for zulip fix 2 (flutter/flutter#173003) 2025-07-30 [email protected] Migrate to null aware elements - Part 4 (flutter/flutter#172322) 2025-07-30 [email protected] Marks Linux linux_feature_flags_test to be unflaky (flutter/flutter#172629) 2025-07-30 [email protected] [ Widget Previews ] Add support for `MultiPreview`s (flutter/flutter#172852) 2025-07-30 [email protected] Made licenses_cpp simpatico with google licenses (flutter/flutter#172991) 2025-07-30 [email protected] Roll Skia from d579722d65c6 to a3347cee2d73 (6 revisions) (flutter/flutter#172989) 2025-07-30 [email protected] Migrate to null aware elements - Part 5 (flutter/flutter#172418) ...
ksokolovskyi
pushed a commit
to ksokolovskyi/flutter
that referenced
this pull request
Aug 19, 2025
…tter#173048) rework of flutter#172701 test: no logical change, just performance change On my machine this takes us from ~35s to 19s. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
mboetger
pushed a commit
to mboetger/flutter
that referenced
this pull request
Sep 18, 2025
…tter#173048) rework of flutter#172701 test: no logical change, just performance change On my machine this takes us from ~35s to 19s. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
korca0220
pushed a commit
to korca0220/flutter
that referenced
this pull request
Sep 22, 2025
…tter#173048) rework of flutter#172701 test: no logical change, just performance change On my machine this takes us from ~35s to 19s. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 12, 2025
lucaantonelli
pushed a commit
to lucaantonelli/flutter
that referenced
this pull request
Nov 21, 2025
…tter#173048) rework of flutter#172701 test: no logical change, just performance change On my machine this takes us from ~35s to 19s. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
rework of #172701
test: no logical change, just performance change
On my machine this takes us from ~35s to 19s.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.