AnOpenSauceDev / FastRandom

A Minecraft Fabric Mod that makes the RNG system of Minecraft 100X faster
Apache License 2.0
48 stars 3 forks source link

Replace `ThreadLocalRandom` with a `L64X128MixRandom` `RandomGenerator` #25

Closed Steveplays28 closed 7 months ago

Steveplays28 commented 7 months ago

This improves performance by 2x, using L64X128MixRandom, compared to vanilla. Also fixed stuck object rendering, such as arrows, displaying inconsistently. Alongside fixes for enchantment and world generation inconsistencies.

AnOpenSauceDev commented 7 months ago

For some reason, I encountered some very unusual spawn behaviour where I was spawned at around y=0 in a mineshaft, but I couldn't get this to occur again, even on the same seed.

AnOpenSauceDev commented 7 months ago

Dropped itemstacks with more than 2 of the same item have some jitter. This does not occur with singular dropped items, and seems to happen at random intervals.

Steveplays28 commented 7 months ago

Seems like thunder is also broken, because the random int used for it never is 0, or the bound that's used is so big that it takes way longer than it should. I'll have to double check if it actually takes longer than vanilla, but I have a feeling it does,

Steveplays28 commented 7 months ago

Just pushed a fix for item rendering. Let me know if there are any other issues!

AnOpenSauceDev commented 7 months ago

The items work fine now, but I just discovered that mineshaft generation is very... chaotic, and seemingly infinite? What's happening is that tonnes of mineshafts are generating next to each other, completely ignoring the structure rules (and greatly reducing chunk generation performance for me). This also affects Badlands Mineshaft generation as well. No other structures are affected. 2024-01-28_08 56 09

Steveplays28 commented 7 months ago

Alright, I'll take a look!

Steveplays28 commented 7 months ago

I think I'll add a new implementation of Minecraft's Random interface for cases where it breaks. That way we can just inject that class into the start of broken methods, replacing the faster random class.

Steveplays28 commented 7 months ago

Hi @AnOpenSauceDev, could you check if the latest commit works without issues for you as well? Mineshafts shouldn't generate everywhere anymore, visual issues should be fixed, and enchantments should be predictable again.

I haven't done any benchmarking yet, could you check how this performs, compared to vanilla, on your system? I'll do some testing as well when I have time.

AnOpenSauceDev commented 7 months ago

Just tried your new commit out, and for some reason the block rotation and item shuffling has returned, and enchantments still seem to be inaccurate. Structures do generate properly again. Not sure if something has gone wrong on my end though.

Steveplays28 commented 7 months ago

Ah, you're right. Also happens on my end now. Weird that it didn't happen for me yesterday.

Steveplays28 commented 7 months ago

I believe I've found a fix that actually works now. It's not as clean as I'd like it to be, since it involves effectively replacing classes, but at least it might work as it should.

My benchmark results seem to be slower than vanilla though? In FasterRandom.onInitialize():

// Benchmark
        var nanoTimeRandomGeneratorRandomStart = System.nanoTime();
        var randomGeneratorRandom = new RandomGeneratorRandom(RandomSeed.getSeed());
        for (int i = 0; i < 100000; i++) {
            randomGeneratorRandom.nextInt();
            randomGeneratorRandom.nextLong();
            randomGeneratorRandom.nextDouble();
            randomGeneratorRandom.nextFloat();
            randomGeneratorRandom.nextBoolean();
            randomGeneratorRandom.nextGaussian();
        }
        var nanoTimeRandomGeneratorRandomEstimated = System.nanoTime() - nanoTimeRandomGeneratorRandomStart;

        var nanoTimeVanillaStart = System.nanoTime();
        var vanillaRandom = new CheckedRandom(RandomSeed.getSeed());
        for (int i = 0; i < 100000; i++) {
            vanillaRandom.nextInt();
            vanillaRandom.nextLong();
            vanillaRandom.nextDouble();
            vanillaRandom.nextFloat();
            vanillaRandom.nextBoolean();
            vanillaRandom.nextGaussian();
        }
        var nanoTimeVanillaEstimated = System.nanoTime() - nanoTimeVanillaStart;

        LOGGER.info("RandomGeneratorRandom: {}s, vanilla: {}s", nanoTimeRandomGeneratorRandomEstimated / 1e9,
                nanoTimeVanillaEstimated / 1e9
        );

With L128X128MixRandom, this returns RandomGeneratorRandom: 0.0229267s, vanilla: 0.0173085s. It's supposed to be 4x faster than vanilla (CheckedRandom). Is my benchmarking code bad?

AnOpenSauceDev commented 7 months ago

With L128X128MixRandom, this returns RandomGeneratorRandom: 0.0229267s, vanilla: 0.0173085s. It's supposed to be 4x faster than vanilla (CheckedRandom). Is my benchmarking code bad?

It could be that one or more random functions are much slower compared to vanilla, so it would be better to benchmark every method separately, instead of doing it all at once.

AnOpenSauceDev commented 7 months ago

Odd, the new generator is consistently slower vs. vanilla.

# int
RandomGeneratorRandom: 0.019367281s, vanilla: 0.007450953s

# long
RandomGeneratorRandom: 0.025516993s, vanilla: 0.012274628s

# double
RandomGeneratorRandom: 0.023700025s, vanilla: 0.010885982s

# float
RandomGeneratorRandom: 0.022416038s, vanilla: 0.00581613s

# bool
RandomGeneratorRandom: 0.019014472s, vanilla: 0.006581874s

# guassian
RandomGeneratorRandom: 0.023590226s, vanilla: 0.023925273s
Steveplays28 commented 7 months ago

huh. This keeps getting weirder. Thanks for testing. I'll ask around in the Fabric Discord server.

AnOpenSauceDev commented 7 months ago

@Steveplays28 Actually, I found a fix. You were accidentally using L128x128MixRandom, which was significantly slower than L64X128MixRandom. After doing that, I got roughly 2x the performance compared to vanilla.

Steveplays28 commented 7 months ago

Oh, nice! Great that you found the issue. Is everything working correctly on your end as well, in terms of all the issues that were present before?

AnOpenSauceDev commented 7 months ago

Yep! Everything works correctly.

Steveplays28 commented 7 months ago

Nice! One last thing I/we could do then, is test if replacing some new CheckedRandom(seed) calls would work. In the world generator, there's quite a few of these instantiations. Might help with gaussian-related functions.

If it does break stuff though, we can always choose not to include it. Is terrain generation currently consistent?

AnOpenSauceDev commented 7 months ago

I'll test it out now, but at a glance worldgen looks like vanilla.

AnOpenSauceDev commented 7 months ago

Can confirm that worldgen is 1:1 when using the same seed.

Steveplays28 commented 7 months ago

Awesome! That's great progress. Do you want to try replacing calls in the world generator? Could do that in a followup PR, alongside another followup PR for a docs update.

I'll mark the PR as ready for review, if you haven't found any other issues. Apologies for all the issues created by my previous PR.

AnOpenSauceDev commented 7 months ago

This improves performance by 2x

Just wrote up a benchmark, it's actually anywhere from 4x -> 8x faster, depending on the generator used.

The new benchmark also a bit more detailed, and only executes on dev.