Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cache analysis options objects in FileStates #54124

Closed
Tracked by #53876
pq opened this issue Nov 21, 2023 · 1 comment
Closed
Tracked by #53876

Cache analysis options objects in FileStates #54124

pq opened this issue Nov 21, 2023 · 1 comment
Labels
analyzer-analysis-options area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-performance Issue relates to performance or code size

Comments

@pq
Copy link
Member

pq commented Nov 21, 2023 •
edited
Loading

Follow-up from https://dart-review.googlesource.com/c/sdk/+/337402.

Storing options pointers in file states will speed up lookups. (This may also do away with any need for a dedicated AnalysisOptionsMap.)
@pq pq mentioned this issue Nov 21, 2023
Open
24 tasks
@pq pq added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on analyzer-analysis-options type-performance Issue relates to performance or code size labels Nov 21, 2023
@pq
Copy link
Member Author

pq commented Dec 11, 2023

This could provide opportunities for #54310.

copybara-service bot pushed a commit that referenced this issue Jan 4, 2024
@pq
options map support scaffolding in driver
717ce7a 
Konstantin and I chatted a bit about this so at least some of it shouldn't be too surprising to him but please do feel free to grab me to chat.

That said a few pointers would be handy. Notably, this change:

* introduces a new `SharedOptionsOptionsMap` for use in SDK drivers (and preserve current semantics)
  * this is currently public but will be private once driver is more evolved to accommodate multiple options files
* removes the shared `_analysisOptions` from the analysis driver with a baby step to using the one shared in SharedOptionsOptionsMap
* removes context_builder and collection v2 experiments (and tests)
  * I'll harvest some more functionality from these in future changes but for now they're distracting and hard to maintain

This work is all to setup moving analysis options awareness into file state objects which will allow us to remove `sdkVersionConstraint` info from options (finally) and a host of other good stuff (see #54124).



Change-Id: Ic4278184016d1018b4b5b1c6ac5ba9e2546927a5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/344860
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>
copybara-service bot pushed a commit that referenced this issue Jan 5, 2024
@pq
Revert "options map support scaffolding in driver"
3e319ba 
This reverts commit 717ce7a.

Reason for revert: breaking Google3

Original change's description:
> options map support scaffolding in driver
>
> Konstantin and I chatted a bit about this so at least some of it shouldn't be too surprising to him but please do feel free to grab me to chat.
>
> That said a few pointers would be handy. Notably, this change:
>
> * introduces a new `SharedOptionsOptionsMap` for use in SDK drivers (and preserve current semantics)
>   * this is currently public but will be private once driver is more evolved to accommodate multiple options files
> * removes the shared `_analysisOptions` from the analysis driver with a baby step to using the one shared in SharedOptionsOptionsMap
> * removes context_builder and collection v2 experiments (and tests)
>   * I'll harvest some more functionality from these in future changes but for now they're distracting and hard to maintain
>
> This work is all to setup moving analysis options awareness into file state objects which will allow us to remove `sdkVersionConstraint` info from options (finally) and a host of other good stuff (see #54124).
>
>
>
> Change-Id: Ic4278184016d1018b4b5b1c6ac5ba9e2546927a5
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/344860
> Reviewed-by: Brian Wilkerson <[email protected]>
> Reviewed-by: Konstantin Shcheglov <[email protected]>
> Commit-Queue: Phil Quitslund <[email protected]>

Change-Id: Ia988c5c9cd328d0adce3d8e6ff019e389819a957
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/344960
Auto-Submit: Phil Quitslund <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue Jan 8, 2024
@pq
Reland "options map support scaffolding in driver" (with a re-introdu…
3224637 
…ced optional `analysisOptions` param)

This is a reland of commit 717ce7a with a slight modification to `AnalysisDriver` to allow Google3 clients to pass an analysisOptions object in.

The logic to accommodate this is only temporary and will go away as soon as I can update clients to use an options map instead.




Original change's description:
> options map support scaffolding in driver
>
> Konstantin and I chatted a bit about this so at least some of it shouldn't be too surprising to him but please do feel free to grab me to chat.
>
> That said a few pointers would be handy. Notably, this change:
>
> * introduces a new `SharedOptionsOptionsMap` for use in SDK drivers (and preserve current semantics)
>   * this is currently public but will be private once driver is more evolved to accommodate multiple options files
> * removes the shared `_analysisOptions` from the analysis driver with a baby step to using the one shared in SharedOptionsOptionsMap
> * removes context_builder and collection v2 experiments (and tests)
>   * I'll harvest some more functionality from these in future changes but for now they're distracting and hard to maintain
>
> This work is all to setup moving analysis options awareness into file state objects which will allow us to remove `sdkVersionConstraint` info from options (finally) and a host of other good stuff (see #54124).
>
>
>
> Change-Id: Ic4278184016d1018b4b5b1c6ac5ba9e2546927a5
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/344860
> Reviewed-by: Brian Wilkerson <[email protected]>
> Reviewed-by: Konstantin Shcheglov <[email protected]>
> Commit-Queue: Phil Quitslund <[email protected]>

Change-Id: Iac9c4eb1aa448f2ca44e32dfb6cdf7cf765b6027
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/344944
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>
copybara-service bot pushed a commit that referenced this issue Jan 12, 2024
@pq
associate analysis options with FileState instances
8b7e1a1 
Preliminary to #54124.

Next step will be to update all lookups to use this new getter.


Change-Id: Ib4a0234ffc8badf906eee4fbd8c363ef4a25adac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345745
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
analyzer-analysis-options area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-performance Issue relates to performance or code size
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@pq