RFR: 8254078: DataOutputStream is very slow post-disabling of Biased Locking [v5]

[Aleksey Shipilev]

On Mon, 12 Oct 2020 18:12:29 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> DataOutputStream is very slow post-disabling of Biased Locking. This
>> was discovered when benchmarking a transaction library, which showed
>> significant performance loss when moving to JDK 15. WIth some small
>> changes to DataOutputStream we can get the performance back. There's a
>> JMH benchmark at
>> http://cr.openjdk.java.net/~aph/JDK-8254078/jmh-tests.tar
>> 
>> Some Stream classes use very fine-grained locking.
>> 
>> In particular, writeInt is defined like this:
>> 
>>         out.write((v >>> 24) & 0xFF);
>>         out.write((v >>> 16) & 0xFF);
>>         out.write((v >>> 8) & 0xFF);
>>         out.write((v >>> 0) & 0xFF);
>>         incCount(4);
>> 
>> Unfortunately, ByteArrayOutputStream.write(byte) is defined like this:
>> 
>>     public synchronized void write(int b) {
>>         ensureCapacity(count + 1);
>>         buf[count] = (byte) b;
>>         count += 1;
>>     }
>> 
>> so we acquire and release a lock for every byte that is output.
>> 
>>  For example, writing 4kb of ints goes from 17.3 us/op to 53.9 us/op when biased locking is disabled:
>> 
>> 
>> +UseBiasedLocking DataOutputStreamTest.dataOutputStreamOverByteArray avgt 6 53.895 ± 5.126 us/op
>> -UseBiasedLocking DataOutputStreamTest.dataOutputStreamOverByteArray avgt 6 17.291 ± 4.430 us/op
>> 
>> There are refactorings of DataOutputStream we can do to mitigate this.
>
> Andrew Haley has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8254078: DataOutputStream is very slow post-disabling of Biased Locking

Benchmark code looks good, except a few remaining nits.

test/micro/org/openjdk/bench/java/io/DataOutputStreamTest.java line 44:

> 42:     @Param({"4096"}) int size;
> 43:     final ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(size);
> 44:     final File f = new File("DataOutputStreamTest.tmp");

This should be `File.createTempFile("DataOutputStreamTest", "out")`? This way you don't leave waste in current dir.

test/micro/org/openjdk/bench/java/io/DataOutputStreamTest.java line 57:

> 55:             bufferedFileStream = new DataOutputStream(new BufferedOutputStream(fileOutputStream));
> 56:             outputString = new String(new byte[size]);
> 57:         } catch (Exception e) {

You can do `public void setup() throws Exception`, and ditch the `try-catch` block. Let JMH deal with exceptions, if
any.

test/micro/org/openjdk/bench/java/io/DataOutputStreamTest.java line 63:

> 61:
> 62:     public void writeChars(DataOutput dataOutput)
> 63:             throws Exception {

Unusual style to put `throws Exception` on the separate line...

-------------

Marked as reviewed by shade (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/542