Avoid unnecessary Context allocations when (re)adding same values#11673
Avoid unnecessary Context allocations when (re)adding same values#11673mcculls wants to merge 1 commit into
Conversation
…dContext/SingletonContext
mcculls
added
comp: core
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08c0b4584d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } else if (secondValue != this.value) { | ||
| return new SingletonContext(this.index, secondValue); | ||
| } else { | ||
| return this; |
There was a problem hiding this comment.
When a caller derives a nested scope with the same key/value, e.g. Context.current().with(k, v).attach(), this now reuses the outer Context instance. With the default ThreadLocalContextManager, ContextScope.close() decides whether a scope is current by context == holder[0]; if the outer scope is closed before the inner one (the out-of-order case covered by ContextManagerTest.testOnlyCurrentScopeCanBeClosed for derived contexts), both scopes now carry the same context, so closing the outer scope incorrectly restores the previous context and detaches the still-active inner scope. Please keep with() returning a distinct context for this case, or make scope-close tracking distinguish scope instances rather than context identity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This issue should be addressed once my other PR is merged, as it introduces no-op scopes when the same context is attached multiple times...
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
1 participant
Motivation
Avoid unnecessary allocations when adding the same key-value to an existing IndexedContext/SingletonContext
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue/merge. You can also:/merge --commit-message "..."/merge -c/merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)Jira ticket: [PROJ-IDENT]