RFR: 8224202: Speed up Properties.load

[Ivan Gerasimov]

Hi Claes!

A few comments:

1)

  571                     if (len == lineBuf.length) {
  572                         int newLength = 
ArraysSupport.newLength(lineBuf.length, 1, lineBuf.length);

could be written slightly shorter as

  571                     if (len == lineBuf.length) {
  572                         int newLength = 
ArraysSupport.newLength(len, 1, len);

2)
  585                     if (len == 0) {
  586                         skipWhiteSpace = true;
  587                         len = 0;

no need to update len here

3)
In loadConvert(char[] in, int off, int len, StringBuilder out):

It may make sense to first scan the input buffer for backslash, and if 
not found (should be a common case), return new String(in, off, len), so 
a copying to the StringBuilder will be avoided.

Otherwise:
     out.append(in, off, posOfSlash - off);
     off = posOfSlash;

and continue as before...

With kind regards,
Ivan


On 5/22/19 3:19 PM, Claes Redestad wrote:
> Hi Roger,
>
> On 2019-05-22 20:09, Roger Riggs wrote:
>> Hi Claes,
>>
>> Properties.java: 542-547, 636-639 you might find the function Ivan 
>> added useful:
>>          newlen =  ArraySupport.newLength(oldlength, min, preferred);
>
> good suggestion - this is a slow path that we don't hit in any of the
> startup tests I'm concerned with, so let's use that.
>
>>
>> Can it be refactored differently to read the bytes into a char array and
>> then process that to avoid the processing code duplication.
>
> .. not without a slowdown.
>
>>
>> If not, can you put the two byte and char versions of readLine in 
>> separate methods,
>> That massive if..else is ugly. Not sharing most of the code doing the 
>> same things is quite ugly too.
>
> ... let's take a step back from that particular ugliness. :-)
>
> http://cr.openjdk.java.net/~redestad/8224202/open.02/
>
> - Joined stream/reader code paths back together
> - Consume comment lines completely - reading new data from
>   stream/reader if we need to - removes the need for the
>   isCommentLine control variable
> - Realized isNewLine is true iff len == 0, so it can be removed
>
> This ends up being almost exactly as fast as open.01, with much
> less code duplication. The break-to-label is unfortunate, of course.
>
> /Claes
>

-- 
With kind regards,
Ivan Gerasimov