Hi Claes,

Properties.java: 542-547, 636-639 you might find the function Ivan added useful:
        newlen =  ArraySupport.newLength(oldlength, min, preferred);

Can it be refactored differently to read the bytes into a char array and
then process that to avoid the processing code duplication.

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.

Roger


On 05/22/2019 01:15 PM, Claes Redestad wrote:
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 comparison in 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

Reply via email to