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.
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
improvement: Request jvmRunEnvironment lazily #6223
Conversation
buildTargetClasses.jvmRunEnvironment | ||
.get(target) | ||
buildTargetClasses | ||
.jvmRunEnvironmentSync(target) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question, can we ask for jvmRunEnvironment here instead? This would remove the need for jvmRunEnvironmentSync
, since jvmRunEnvironment
can return cached results.
Or maybe waiting for the response here would be too long?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather avoid using Await here and using Future propagates it all the way and it becomes quite complex. There are multiple sequences of futures etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you' re caching jvmRunEnvironments
but I don't see you ever updating or clearing that cache.
Goo catch, I added it to |
Previously, we would eagerly request jvmEnvironment for all known main classes, which in case of Bazel might cause a lot of queries. This might also not be efficient in some other cases. Now, we only request and cache jvmRunEnvironment when needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Previously, we would eagerly request jvmEnvironment for all known main classes, which in case of Bazel might cause a lot of queries. This might also not be efficient in some other cases. Now, we only request and cache jvmRunEnvironment when needed.