RFR 8170348: Appendable.appendN(char, int) method to append multiple copies of char

Ivan Gerasimov ivan.gerasimov at oracle.com
Sun Dec 11 20:26:04 UTC 2016


Thank you Paul for the comments and suggestions!


> Add a fix version of 10, then it’s clear on the intent. Once tests are 
> added :-) we can review for that release. 

Yes, right.  I've set the fix version to 10 and also added the @since 10 
tag.
I've added a couple of test cases to 
`test/java/lang/Appendable/Basic.java`, or do you think the appendN() 
method worth more testing?

> Personally i would use Arrays.fill, i gather the pattern is already recognised:
>
> https://bugs.openjdk.java.net/browse/JDK-4809552
> http://hg.openjdk.java.net/hsx/hsx19/baseline/rev/d64a8c7aa9d5
>
> The advantage of using Arrays.fill is we know that the pattern will be recognized (if not it’s a bug, and i suppose it could be made intrinsic to wire up faster). (see -XX:+TraceOptimizeFill). I suspect we should review the filling optimization to see if it can be enhanced with newer SIMD instructions, as i gather the current implementation writes a max of 8 bytes at a time (with an explicit unrolled loop for 32 bytes at a time, containing 4 separate stores).

Okay, I replaced the first loop with a call to Arrays.fill().
Benchmark shows that for size == 1 the overhead is bigger then for 
adding exactly one char in a loop.
For anything longer it appears to be faster.

Benchmark                 (size)  Mode  Cnt  Score   Error Units
MyBenchmark.test_0_New         0  avgt   85  0.004 ± 0.001  us/op
MyBenchmark.test_0_New         1  avgt   85  0.016 ± 0.003  us/op
MyBenchmark.test_0_New         5  avgt   85  0.017 ± 0.003  us/op
MyBenchmark.test_0_New        10  avgt   85  0.020 ± 0.004  us/op
MyBenchmark.test_0_New        20  avgt   85  0.021 ± 0.004  us/op
MyBenchmark.test_1_Old         0  avgt   85  0.004 ± 0.001  us/op
MyBenchmark.test_1_Old         1  avgt   85  0.007 ± 0.001  us/op
MyBenchmark.test_1_Old         5  avgt   85  0.029 ± 0.006  us/op
MyBenchmark.test_1_Old        10  avgt   85  0.048 ± 0.010  us/op
MyBenchmark.test_1_Old        20  avgt   85  0.099 ± 0.019  us/op
MyBenchmark.test_2_Solid       0  avgt   85  0.008 ± 0.001  us/op
MyBenchmark.test_2_Solid       1  avgt   85  0.015 ± 0.002  us/op
MyBenchmark.test_2_Solid       5  avgt   85  0.026 ± 0.005  us/op
MyBenchmark.test_2_Solid      10  avgt   85  0.031 ± 0.004  us/op
MyBenchmark.test_2_Solid      20  avgt   85  0.028 ± 0.005  us/op

>
> It’s good that you found places in the JDK, that adds justification for such methods, especially for BigInteger and that static array of strings full of ‘0’ characters.
>
> The updates to MethodHandleImpl are not perf sensitive, i question the usage here but it does reduce stuff in the constant pool i suppose.
I think it also improves readability, as it's clear now the amount of 
spaces added.


With kind regards,
Ivan



More information about the core-libs-dev mailing list