[ 
https://issues.apache.org/jira/browse/PROTON-1916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16588767#comment-16588767
 ] 

ASF GitHub Bot commented on PROTON-1916:
----------------------------------------

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.


> Makes StringsBenchmark::encodeStringMessage GC free
> ---------------------------------------------------
>
>                 Key: PROTON-1916
>                 URL: https://issues.apache.org/jira/browse/PROTON-1916
>             Project: Qpid Proton
>          Issue Type: Improvement
>            Reporter: Francesco Nigro
>            Priority: Trivial
>             Fix For: proton-j-0.30.0
>
>
> StringsBenchmark is allocating a new ByteBufferWrapper instance
> on each encodeStringMessage invocation, making the encoding results depending 
> by
> the amount of garbage produced: turning the benchmark GC free will allow to 
> obtain more
> stable results and focus just on the String encoding speed.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to