Skip to content

Improve the performance of accessing ThreadedBlockData.#1417

Open
copi143 wants to merge 41 commits intoGTNewHorizons:masterfrom
copi143:cache-threadlocal
Open

Improve the performance of accessing ThreadedBlockData.#1417
copi143 wants to merge 41 commits intoGTNewHorizons:masterfrom
copi143:cache-threadlocal

Conversation

@copi143
Copy link
Copy Markdown
Contributor

@copi143 copi143 commented Feb 23, 2026

For functions that need to access fields inside ThreadedBlockData multiple times, cache the ThreadedBlockData object at the start of the function instead of retrieving it from the ThreadLocal each time.

@Alexdoru
Copy link
Copy Markdown
Member

doesn't work

image

@copi143
Copy link
Copy Markdown
Contributor Author

copi143 commented Feb 24, 2026

Yes. analyzer.analyze can sometimes throw odd exceptions. On my machine, a subset of classes consistently fail to be handled correctly, so I added an analyzeSuccess variable to fall back to the original implementation on failure.

@copi143
Copy link
Copy Markdown
Contributor Author

copi143 commented Feb 24, 2026

Oh no. It seems there are other strange issues as well. I'll try to fix it, overall it shouldn't be worse than before for now.

final boolean changed = inner.transformClassNode(transformedName, cn);
if (changed) {
final ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS);
final ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's gonna need a safe class writer to not blow up

Alexdoru and others added 2 commits February 28, 2026 23:42
…leList and ImmutableMap for immutable data, and make overloadedMethods use ConcurrentHashMap to avoid concurrency issues.
@copi143
Copy link
Copy Markdown
Contributor Author

copi143 commented Mar 1, 2026

Now analyzer.analyze will no longer run for a large number of unrelated methods.

@Alexdoru
Copy link
Copy Markdown
Member

Alexdoru commented Mar 2, 2026

your implementation always injects at the head of the method which will crash if the method is a constructor
image

@Alexdoru
Copy link
Copy Markdown
Member

Alexdoru commented Mar 2, 2026

you need to increment the local variable name in case it injects multiple local variables
image

@Alexdoru
Copy link
Copy Markdown
Member

Alexdoru commented Mar 2, 2026

is it possible to move the local variable allocation closer to where it's used to avoid cases where it will get the data but not use it
image

@copi143
Copy link
Copy Markdown
Contributor Author

copi143 commented Mar 3, 2026

you need to increment the local variable name in case it injects multiple local variables image

Actually, incrementing names aren't necessary because I'm using unique IDs.
The name is just for easier decompilation and debugging.

@copi143
Copy link
Copy Markdown
Contributor Author

copi143 commented Mar 3, 2026

is it possible to move the local variable allocation closer to where it's used to avoid cases where it will get the data but not use it image

This would be quite complicated, and the following situations might occur:

if (xxx) {
    cache = ...
    cache.x
    cache.y
}
cache.z

};
// Needed because the config is loaded in LaunchClassLoader, but we need to access it in the parent system loader.
private static final MethodHandle angelicaConfigCeleritasEnabledGetter;
private static boolean isCeleritasEnabled;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need most of the reorganization changes in your pr now, it already got refactored in master, so you don't need to reorder fields and to keep methods like isBlockSubclass, trackBlockSubclasses and trackBlockShadowingFields

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants