search

McModLauncher / modlauncher

Java 21 mod launcher
Other
171 stars 47 forks source link

Use ConcurrentHashMaps, fixes #42 #43

Closed ichttt closed 4 years ago

ichttt commented 4 years ago

Microbenchmark results: With traditional HashMap: Benchmark Mode Cnt Score Error Units TransformBenchmark.transformDummyClass avgt 9 10,990 ± 3,939 us/op TransformBenchmark.transformDummyClass:·stack avgt NaN --- TransformBenchmark.transformNoop avgt 9 4,309 ± 2,177 us/op TransformBenchmark.transformNoop:·stack avgt NaN ---

With concurrent HashMap: Benchmark Mode Cnt Score Error Units TransformBenchmark.transformDummyClass avgt 9 11,001 ± 3,756 us/op TransformBenchmark.transformDummyClass:·stack avgt NaN --- TransformBenchmark.transformNoop avgt 9 4,386 ± 2,302 us/op TransformBenchmark.transformNoop:·stack avgt NaN ---

Note that the error margin is very high, as it looks like the garbage collector is the actual bottleneck here, as my CPU was maxed out fairly often despite this being a single-threaded benchmark.

Forge startup time does not seem to change when using ConcurrentHashMaps Note: this fix may not be complete, there is some weird stuff going on that may not be completly thread-safe

Raycoms commented 4 years ago

Any update on this?

Raycoms commented 4 years ago

For anyone reading this, citing lex on this:

because the performance isnt negligible esp in all the hacky shit we have to deal.with
because least time we tried this it made load time 15x longer in real.worls test
because last time we tried this it caused deadlocks whenncoremods were being stupid
because last time we tried this exact fucking thing it FUCKED EVERYTHIG
cpw commented 4 years ago

Raycoms, this is in a different context: we can't use CHM in eventbus. This may be less impactful. I'll review...

cpw commented 4 years ago

96562ea62aee6a2e22780c5ad6dc37c4057a67b0