-
Notifications
You must be signed in to change notification settings - Fork 86
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
Update WireHelpers.java to improve data-blob copying by bulk put #143
Conversation
i was thinking of using put(byte[] int, int length) instead, since the buffers' positions will be changed. |
Looks good to me. Thanks! |
@dwrensha what about the other functions with the same code? Also, is it thread safe? Because I am changing the ByteBuffer position? |
I opened #144 for the other TODOs. |
perf, thanks! |
Hm... setTextPointer uses capnproto-java/runtime/src/main/java/org/capnproto/WireHelpers.java Lines 785 to 787 in 08009f3
I suppose that is more thread-safe? |
I just pushed 28ab5ce |
I added a test in cfea947. The |
Ah thanks 👍, I tried avoiding the duplicate copying but it makes sense I see! |
For the record, |
Hello maintainers!
For context: I am using capnproto-java for image processing pipelines in kotlin on android. For this matter, there is a lot of ByteArrays for data buffers of image blobs.
Now while profiling I saw that the data copying has some nasty CPU-usage:
Almost a third of the copy operation is checkIndex().
And in the source code I saw this TODO.
I have seen that bulk ByteBuffer.put(byte[], int, int) and ByteBuffer.put(ByteBuffer serc) is available since Java 8. Is there a reason not to for minimal Java version?
I see the same issue in the code with getWritableDataPointer without taking a closer look.
Cheers
Steve