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

Avoid unnecessary array copy in singlePass #408

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Conversation

pengxiaolong
Copy link
Contributor

@pengxiaolong pengxiaolong commented Oct 17, 2024

Issue #, if available: #407

Description of changes:

Both java_buffer::from_array and GetByteArrayRegion support consuming a slice of the array by passing offset and length, the change adds a bufOffset arguments to fastDigest to avoid unnecessary array copy in singlePass which needs to invoke fastDigest method.

We have tested the fix with JMH:

    private static ThreadLocal<MessageDigest> messageDigest = new ThreadLocal<MessageDigest>() {
        protected synchronized MessageDigest initialValue() {
            MessageDigest md = null;
            try {
                md = MessageDigest.getInstance("md5");
            } catch (NoSuchAlgorithmException e) {
                throw new RuntimeException("cannot find md5 algorithm", e);
            }
            return md;
        }
    };

    public static byte[][] inputs = new byte[][]{
            "1234567890123456789012345678901212345678901234567890123456789012".getBytes(), // 64 bytes
            "12345678901234567890123456789012".getBytes(),
            "12345678901234567890123456789012".getBytes(),
            "12345678901234567890123456789012".getBytes(),
            "12345678901234567890123456789012".getBytes(),
            "12345678901234567890123456789012".getBytes(),
            "12345678901234567890123456789012".getBytes(),
            "12345678901234567890123456789012".getBytes()
    };

    @Benchmark
    public static void test() {
        MessageDigest md = messageDigest.get();
        for(byte[] input : inputs) {
            md.digest(input);
        }
    }

JMH result:
W/o fix:

Benchmark                                  Mode  Cnt     Score    Error   Units
ACCPMD5Benchmark.test                      avgt    5  1630.452 ± 97.567   ns/op
ACCPMD5Benchmark.test:·gc.alloc.rate       avgt    5   346.316 ± 20.570  MB/sec
ACCPMD5Benchmark.test:·gc.alloc.rate.norm  avgt    5   592.000 ±  0.001    B/op
ACCPMD5Benchmark.test:·gc.count            avgt    5    11.000           counts
ACCPMD5Benchmark.test:·gc.time             avgt    5    11.000               ms

W/ fix:

Benchmark                                  Mode  Cnt     Score    Error   Units
ACCPMD5Benchmark.test                      avgt    5  1583.675 ± 94.346   ns/op
ACCPMD5Benchmark.test:·gc.alloc.rate       avgt    5   154.179 ±  9.225  MB/sec
ACCPMD5Benchmark.test:·gc.alloc.rate.norm  avgt    5   256.000 ±  0.001    B/op
ACCPMD5Benchmark.test:·gc.count            avgt    5     4.000           counts
ACCPMD5Benchmark.test:·gc.time             avgt    5     4.000               ms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@pengxiaolong pengxiaolong requested a review from a team as a code owner October 17, 2024 17:39
Copy link
Contributor

@amirhosv amirhosv left a comment

Choose a reason for hiding this comment

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

Thanks for catching this.

@geedo0 geedo0 merged commit 5818fbf into corretto:main Oct 18, 2024
11 checks passed
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.

3 participants