Skip to content

Add JMH benchmarks, improve storage performance of StorageService#getAvailableStacks >50% - #8904

Open
gharris1727 wants to merge 4 commits into
AppliedEnergistics:forge/1.20.1from
gharris1727:improve-storage-perf
Open

Add JMH benchmarks, improve storage performance of StorageService#getAvailableStacks >50%#8904
gharris1727 wants to merge 4 commits into
AppliedEnergistics:forge/1.20.1from
gharris1727:improve-storage-perf

Conversation

@gharris1727

@gharris1727 gharris1727 commented Jun 21, 2026

Copy link
Copy Markdown

TL;DR: This patch includes performance improvements of 50-200% for AE2 server tick times.

On our recent Monifactory playthrough, we noticed our endgame ME network was a substantial performance hog on server resources, bringing us down to ~9 TPS. Most of the time was spent in onServerEndTick, processing the contents of the networks in the StorageService.

I wrote some microbenchmarks to target these codepaths using the existing API, made changes to the implementation to optimize those benchmarks, wrote a script to summarize those improvements as a graph, and manually validated the change by applying it to our server to achieve 13-20 TPS.

Here's the graph summarizing the performance gains:
Screenshot_2026-06-20_18-28-35

To run individual benchmarks (takes ~5 minutes) and experiment with the changes directly and see flamegraphs, use ./gradlew jmh
To run the full suite (takes ~1.5 hours) use node scripts/run_jmh_tests.mjs.
To generate the graph as an svg, use node scripts/generate_report.mjs.
All of the results are in build/results/jmh/.

Some lingering concerns I have:

  • I believe I did not make any correctness changes, but the testing infrastructure for the StorageService appears a bit lacking, and the algorithmic changes there are the most risky
  • Because the benchmark relies on only vanilla items (~1600 total), the benchmarks are biased towards networks with a lot of items that can take damage and enchantments. I suspect performance gains with more typical bases will be different, maybe better.
  • I experimented with some other performance improvements beyond these three commits, but their benefits were more ambiguous. I dropped them from this PR in favor of just the big obvious wins, but we can follow up with more improvements later with this testing infrastructure.
  • I'm developing against the forge branch, but I tried cherry-picking these changes to main and while there are some conflicts, overall this performance problem is still present.

@shartte

shartte commented Jun 21, 2026

Copy link
Copy Markdown
Member

My gut feeling on this topic has been: we should not build an index on secondary key values (damage) at all unless there's actually an observer that requires that information (meaning an export bus or level that uses a fuzzy upgrade)
It's hard to substantiate stats on this but I'd say many of the larger nets don't actually use fuzzy cards.

@shartte

shartte commented Jun 21, 2026

Copy link
Copy Markdown
Member

I am also sorry, we're not going to merge this in 1.20.1

@gharris1727

Copy link
Copy Markdown
Author

Thanks @shartte for looking at this.

we should not build an index on secondary key values (damage) at all unless there's actually an observer that requires that information

This sounds pretty reasonable to me, and the flame graphs show a lot of time spent in the AVL tree code. Do you want me to explore this in this initial optimization pass, in a follow-up pr, or explore it yourself?

I am also sorry, we're not going to merge this in 1.20.1

Would you like me to retarget this PR at main? I can fix the minor merge conflicts I saw and re-run the benchmarks.

@gharris1727

gharris1727 commented Jun 23, 2026

Copy link
Copy Markdown
Author

I looked into porting this change to main, and I'm running into a problem: I want to use Items in the JMH tests, but the only way to get Items seems to be to have minecraft bootstrapped. It looks like the infrastructure is already built to bootstrap minecraft for JUnit tests, but I can't find something similar for JMH.

I see broadly two ways forward:

  • Figure out how to bootstrap minecraft in JMH tests (e.g. by creating an FMLLoader/Bootstrap/etc)
  • Figure out how to abstract AE2 to permit testing without bootstrapped minecraft. (e.g. subclass of AEKey)

I'll keep investigating these to try and get these tests runnable on main.

The only other NeoForge mod i'm aware of that uses JMH appears to do so without depending on bootstrapping in any of its tests: https://github.com/neoforged/Bus/tree/main/bus-jmh/src/main/java/net/neoforged/bus/benchmarks . So that would be a point in favor of the second strategy.

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.

2 participants