You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by franz1981 <gi...@git.apache.org> on 2018/08/21 16:49:02 UTC
[GitHub] qpid-proton-j pull request #15: PROTON-1916: Makes StringsBenchmark::encodeS...
GitHub user franz1981 opened a pull request:
https://github.com/apache/qpid-proton-j/pull/15
PROTON-1916: Makes StringsBenchmark::encodeStringMessage GC free
It includes a perf improvement on string encoding to simplify
the JVM work to compute bounds checking by using a specific
implementation of string encoding for heap ByteBuffers
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/franz1981/qpid-proton-j PROTON-1916
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/qpid-proton-j/pull/15.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #15
----
commit 24f46a124951c8b45ec6a8c67ab1adc1b07c41d7
Author: Francesco Nigro <ni...@...>
Date: 2018-08-21T16:21:59Z
PROTON-1916: Makes StringsBenchmark::encodeStringMessage GC free
It includes a perf improvement on string encoding to simplify
the JVM work to compute bounds checking by using a specific
implementation of string encoding for heap ByteBuffers
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-proton-j issue #15: PROTON-1916: Makes StringsBenchmark::encodeStringMe...
Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on the issue:
https://github.com/apache/qpid-proton-j/pull/15
We don't get notified of updated pushes for the mirrors, so I didn't know you had modified this since my comment.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-proton-j pull request #15: PROTON-1916: Makes StringsBenchmark::encodeS...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/qpid-proton-j/pull/15
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-proton-j issue #15: PROTON-1916: Makes StringsBenchmark::encodeStringMe...
Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on the issue:
https://github.com/apache/qpid-proton-j/pull/15
The JIRA, PR, and commit say they are primarily changing the benchmark/test but most of the change has nothing to do with that. Even though the perf change is tiny and most things wont see it (due to them implementing their own WritableBuffer and not using the method updated at all) its really the main change here so things should reflect that.
The benchmark updated was doing what was originally desired, but the updated version is more specifically targeting the String encoding side of things, so no issue with changing that. The commenting-out of the decode benchmark however seems like it should be removed though.
The main updates change the bracing style of half the class, making it a lot harder to see what you actually changed than it needs to be, and then becoming inconsistent with the rest of the class. Please restore things to the existing format to minimise the diff.
You have introduced a full new method that nothing apparently ever uses, since a non-array buffer isn't passed to it. I suppose there was a line untested before for this to be the case, but it was far simpler and there would now be a 50 line method in addition, so I think adding some basic testing is needed for that.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org