[diffs] WorkerPool & Element Pool Hardening#737
Merged
Conversation
This will ensure immediate plaintext updates
When it cames to render options updating, there were a ton of potential race conditions and extra delays that were incurred, this should fix most of them. * Proper invalidate render option updates if they overlap * Ensure that invalid highlighting jobs are discarded
There were a variety of race conditions where an element could be pooled and re-claimed when it's shadow dom was actually invalid. This should fix all of those cases from before Also improved the max size for the element tool to dynamically adjust to possible render windows to reduce general unnecessary pooling thrash.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
amadeus
changed the title
amadeus
commented
May 23, 2026
| item.instance.primeHighlightCache(); | ||
| } | ||
|
|
||
| private getElementPoolLimit() { |
Member
Author
There was a problem hiding this comment.
Another smol optimization that went into here -- in fully collapsed diff views, i was noticing that we were blowing our max pool windows constantly, so I reworked the max size to better fit the max collapsed items we might render. Also made it larger for isContainerManaged which actually means elements need an extra tick to be cleaned up before they can be re-cycled.
amadeus
added a commit
that referenced
this pull request
May 24, 2026
* Manually clear renderCache on theme change for WorkerPool This will ensure immediate plaintext updates * Fix a ton of WorkerPoolManager race conditions When it cames to render options updating, there were a ton of potential race conditions and extra delays that were incurred, this should fix most of them. * Proper invalidate render option updates if they overlap * Ensure that invalid highlighting jobs are discarded * Fix poisoned element pool There were a variety of race conditions where an element could be pooled and re-claimed when it's shadow dom was actually invalid. This should fix all of those cases from before Also improved the max size for the element tool to dynamically adjust to possible render windows to reduce general unnecessary pooling thrash.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Basically the theme switcher branch for diffshub has exposed a ton of flaws in my WorkerPoolManager and element pooling architecture that are essentially fixed here.
The fixes are grouped into 3 commits that address a few important scenarios.
Clean
renderCacheon worker pool theme changeWhen the worker pool has a theme change, we need to blow away our existing render cache to ensure we never render from a stale theme. Additionally this helps the theme change feel instant/responsive because we can render plain text right away and not wait for a response from the worker.
WorkPoolManager.setRenderOptionsrace conditionsThe core change here was that given the async nature of loading/setting themes (both to load the theme and update the workers), we needed better validation under rapid theme changes to ensure that tasks are properly dropped and if things come back out of order they can be properly ignored.
Fixing poisoned elements
While we were clearing the element pool correctly on theme changes, there was another bug wherein if we were recycling on that tick we were then pooling invalid elements. So this added a sort of version tracker for the element pool that gets incremented and tied to versioned renders to ensure that pooled element contract is always correct with its children