Fwd: Re: RFR: 8224202: Speed up Properties.load

[Sean Mullan]

FYI, in case you are not subscribed to core-libs-dev.

--Sean


-------- Forwarded Message --------
Subject: Re: RFR: 8224202: Speed up Properties.load
Date: Wed, 22 May 2019 19:15:07 +0200
From: Claes Redestad <claes.redestad at oracle.com>
To: core-libs-dev <core-libs-dev at openjdk.java.net>

Reworked after further analysis:

- swapping out char[] for StringBuilder was only a win for the outgoing
   buffer
- using locals for oft-accessed variables in readLine is a substantial
   interpreter-only optimization[1]
- the skipLF logic can safely be folded into the end-of-line logic,
   thus removing a test from every non-comment character read
- splitting apart the InputStream / Reader removes numerous branches
   and allows some improvements for the common InputStream case - at the
   price of some code duplication

In total, this patch now means we need to execute almost half as much
bytecode to load java.security. In my local tests this means we load
java.security ~1.5-2ms faster.

New webrev: http://cr.openjdk.java.net/~redestad/8224202/open.01/

/Claes

[1] this optimization effort is motivated by startup regressions caused
by loading java.security earlier during bootstrap in some
configurations; the number of property files we load is typically small
and happen early, so optimization focus is thus on making it faster
during bootstrap, i.e., interpreted performance - if we were optimizing
for parsing many property files we'd probably need to restructure the
logic, e.g., splitting out duplicate code into smaller methods etc..


On 2019-05-21 12:22, Claes Redestad wrote:
> Hi,
> 
> a few smaller optimizations for Properties.load:
> 
> - when parsing comment lines, we unnecessarily breaks the fast path loop
> on backslashes, even though a backslash embedded in a comment line will 
> never have any effect on subsequent logical lines
> 
> - inside that same loop, we always do two comparisons in the common 
> case: testing c <= '\r' && c >= '\n' would mean only one comparisonin 
> the common case, since c is very likely to be > '\r' (also when reading 
> from a byte stream)
> 
> - we currently store everything into temporary char[]'s which we then
> instantiate Strings with. Since Compact String this means we often
> use more temporary storage than necessary, and do unnecessary work
> packing from char[] to byte[] representation internally; by using
> StringBuilder encoding transitions is handled more gracefully and is a
> speed-up in general (both interpreted and compiled) while simplifying
> the code somewhat.
> 
> Result is ~5-20% faster depending on how comment-heavy the properties
> file we're loading is.
> 
> Bug:    https://bugs.openjdk.java.net/browse/JDK-8224202
> Webrev: http://cr.openjdk.java.net/~redestad/8224202/open.00/
> 
> Testing: tier1-3
> 
> Patch is applied on top of JDK-8224240, patch for which is out for
> review here: 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060301.html
> 
> Thanks!
> 
> /Claes