Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MathUtils.fnv1a_32 is very expensive #44

Closed
gaurav-rana opened this issue Mar 27, 2024 · 2 comments
Closed

MathUtils.fnv1a_32 is very expensive #44

gaurav-rana opened this issue Mar 27, 2024 · 2 comments

Comments

@gaurav-rana
Copy link
Contributor

We did CPU profiling by integrating GB java client SDK into our backend service and are making 100 calls to GB.evalFeature() per request (assuming in prod we'll run 100 experiments in parallel per request), we observed GB taking too much CPU, we narrowed it down to MathUtils.fnv1a_32 where it's spending 75% of overall CPU inside GB. As you can see in the attached screenshot, most of the time is being spent inside the hash function.
Our overall application CPU increased drastically because of this
Is there any faster hashing strategy that we can use here?
Screenshot 2024-03-26 at 5 13 22 PM

I suspect usage of BigInteger in fnv1a_32 is too expensive. Will moving to integer (instead of BigInteger) solve for this issue?

private static final int FNV_32_PRIME = 0x01000193;
private static final int FNV_32_INIT = 0x811c9dc5;

public static int fnv1a_32_int(byte[] data) {
        int hash = FNV_32_PRIME;
        for (byte b : data) {
            // XOR the bottom with the current octet.
            hash ^= (b & 0xff);
            // Multiply by the 32 bit FNV magic prime mod 2^32
            hash *= FNV_32_INIT;
        }
        return hash;
    }
@gaurav-rana
Copy link
Contributor Author

We did the profiling again by removing the dependency on BigInteger and use integer instead (PR: https://github.com/growthbook/growthbook-sdk-java/pull/45/files)
We observed that the performance issue is fixed, the CPU usage in our environment for GrowthBookUtils.hashV2() dropped from 11.25% to 0.13%
but the test cases start to fail (because a specific set of hashing outcomes are defined in the test case)
Screenshot 2024-03-26 at 11 43 29 PM

@vazarkevych
Copy link
Collaborator

vazarkevych commented May 3, 2024

We suggest using xor, multiply and mod operations on long type instead of BigInteger
#50

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

No branches or pull requests

2 participants