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

Refactor use System.arraycopy() to copy array. #6445

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

remeio
Copy link
Contributor

@remeio remeio commented Apr 21, 2023

No description provided.

@codeboyzhou
Copy link

I have a question about why should we use System.arraycopy() in this code snippet, for most small arrays, e.g. length less than 50, for-loop is generally faster than system array copy according to benchmark test.

@kevinb9n
Copy link
Contributor

That's unlikely; got a citation for that benchmark?

@remeio remeio closed this Apr 25, 2023
@kevinb9n
Copy link
Contributor

Not sure the change was a bad idea. (Not sure it's worth changing either, but...)

@kevinb9n kevinb9n reopened this Apr 25, 2023
@remeio
Copy link
Contributor Author

remeio commented Apr 25, 2023

For case 1:
'x' -> "length less than 50".toCharArray(), the for-loop is better.
For case 2:
'y' -> "length is too long, .................................................".toCharArray(), the System#arrayCopy is better.
Such as.

CharEscaper charEscaper =
        createSimpleCharEscaper(
            ImmutableMap.<Character, char[]>builder()
                .put('x', "length less than 50".toCharArray())
                .put('y', "length is too long, .................................................".toCharArray())
                .put('z', "<lo>".toCharArray())
                .buildOrThrow());
    UnicodeEscaper unicodeEscaper = Escapers.asUnicodeEscaper(charEscaper);
    unicodeEscaper.escape("xxxxxxx")

Usually, the case 1 is better way, do you think the case 2 is necessary?

@codeboyzhou
Copy link

It's a little pity that I didn't preserve the benchmark test process and result, but when I continue to run it much more times I start to find that the result is not always like what I am thinking, it looks like unstable, sometimes the system.arraycopy() is faster, but occasionally slower.

I'm having some doubts, maybe my personal computer or environment has impact on the test result, or my test cases are not completely exact, not sure about that.

But anyway, at least I think system.arraycopy() is more brief, although this sounds like not very persuasive. :)

@abimarank
Copy link

System.arraycopy() should be faster as it's a native call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants