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