RFR (S) 8145428: Optimize StringUTF16 compress/copy methods for C1

Roger Riggs Roger.Riggs at Oracle.com
Thu Dec 17 16:54:21 UTC 2015


Hi Aleksey,

I should have worded it as a suggestion (and provided the words) so as 
avoided impeding progress.
It could have been as general as saying that an @implNote that String is 
uniquely performance sensitive,
that would apply to the whole file, not just a few expressions, and it 
should probably have been there
for 20 years.

You are particularly sensitive to performance, not every developer is 
and they assume
that the compiler does reasonable optimizations (it doesn't) and that 
what look
like equivalent expressions should work the same.  As the comments to 
your original change
elicited, the change is tuning the source code for a particular 
optimization (or lack of) in C1.

I agree that you should just push it and move on.

Roger


On 12/17/2015 11:37 AM, Aleksey Shipilev wrote:
> Hi Roger,
>
> Um, do you really want to spell out "thou shalt run thee benchmarks
> while changing the String hotpath" in source code comments?
>
> I disagree: documenting development processes in the source code is odd.
> But equally odd is the suggestion that a common development practice
> would *not* involve running benchmarks when changing the ubiquitous
> codepath in the standard library, *or* studying the prior history for
> the code under modification. Some knowledge really is common.
>
> Can we pretty please get this done, and move on to other interesting things?
>
> Thanks,
> -Aleksey
>
> On 12/17/2015 05:55 PM, Roger Riggs wrote:
>> Hi Alexsey,
>>
>> The 'expected to run benchmarks' might the operative comment in the code.
>> 'Common' knowledge sometimes isn't so common.
>>
>> Roger
>>
>>
>> On 12/17/2015 2:54 AM, Aleksey Shipilev wrote:
>>> On 12/17/2015 02:34 AM, Ulf wrote:
>>>> I'm wondering why moving the increment operation to an extra line wound
>>>> enhance performance.
>>> Because C1 is very straightforward, and code movement like that is a
>>> poor man's instruction scheduling, that pads out the data dependency
>>> between index update and indexed access. I don't think it deserves a
>>> comment -- it is expected one will run the benchmarks when changing that
>>> code.
>>>
>>> Thanks,
>>> -Aleksey
>>>
>




More information about the core-libs-dev mailing list