RFR: 8263560: Remove needless wrapping with BufferedInputStream

[Сергей Цыпанов-2]

In some cases wrapping of `InputStream` with `BufferedInputStream` is redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does not require any buffer having one within.

Other cases are related to reading either a byte or short `byte[]`: in both cases `BufferedInputStream.fill()` will be called resulting in load of much bigger amount of data (8192 by default) than required.

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

Commit messages:
 - Use InputStream.readNBytes()
 - Drop unnecessary BufferedInputStream-wrapping where possible

Changes: https://git.openjdk.java.net/jdk/pull/2992/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2992&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263560
  Stats: 14 lines in 3 files changed: 2 ins; 7 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2992.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2992/head:pull/2992

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

[Сергей Цыпанов-2]

On Sun, 14 Mar 2021 07:51:24 GMT, Alan Bateman <[hidden email]> wrote:

>> In some cases wrapping of `InputStream` with `BufferedInputStream` is redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does not require any buffer having one within.
>>
>> Other cases are related to reading either a byte or short `byte[]`: in both cases `BufferedInputStream.fill()` will be called resulting in load of much bigger amount of data (8192 by default) than required.
>
> src/java.base/share/classes/jdk/internal/jmod/JmodFile.java line 57:
>
>> 55:             // validate the header
>> 56:             byte[] magic = new byte[4];
>> 57:             in.read(magic);
>
> While you are there, this can be changed to magic = in.readNBytes(4) and throw IOException if magic.length != 4.

Done.

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

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

[Сергей Цыпанов-2]

On Sun, 14 Mar 2021 17:40:27 GMT, Alan Bateman <[hidden email]> wrote:

>> It turned out, that with this latest update some of tier2_tools tests are failing, e.g. `JLinkNegativeTest`:
>> test JLinkNegativeTest.testMalformedJmod(): failure
>> java.lang.AssertionError: Output does not fit regexp: Error: java.io.IOException: Invalid JMOD file
>> at tests.Result.assertFailure(Result.java:70)
>> at JLinkNegativeTest.testMalformedJmod(JLinkNegativeTest.java:201)
>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
>> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>> at java.base/java.lang.reflect.Method.invoke(Method.java:568)
>> at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85)
>> at org.testng.internal.Invoker.invokeMethod(Invoker.java:639)
>> at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821)
>> at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1131)
>> at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:125)
>> at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108)
>> at org.testng.TestRunner.privateRun(TestRunner.java:773)
>> at org.testng.TestRunner.run(TestRunner.java:623)
>> at org.testng.SuiteRunner.runTest(SuiteRunner.java:357)
>> at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352)
>> at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310)
>> at org.testng.SuiteRunner.run(SuiteRunner.java:259)
>> at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
>> at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
>> at org.testng.TestNG.runSuitesSequentially(TestNG.java:1185)
>> at org.testng.TestNG.runSuitesLocally(TestNG.java:1110)
>> at org.testng.TestNG.run(TestNG.java:1018)
>> at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94)
>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
>> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>> at java.base/java.lang.reflect.Method.invoke(Method.java:568)
>> at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298)
>> at java.base/java.lang.Thread.run(Thread.java:831)
>> ...
>> java.lang.module.FindException: java.io.IOException: Invalid JMOD file: /home/s.tsypanov/IdeaProjects/jdk-github/build/linux-x86_64-server-release/test-support/jtreg_test_jdk_tier2/scratch/3/jmods/hacked4.jmod
>> at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:253)
>> at java.base/jdk.internal.module.ModulePath.scanNextEntry(ModulePath.java:190)
>> at java.base/jdk.internal.module.ModulePath.find(ModulePath.java:154)
>> at java.base/java.lang.module.Resolver.findWithBeforeFinder(Resolver.java:842)
>> at java.base/java.lang.module.Resolver.resolve(Resolver.java:119)
>> at java.base/java.lang.module.Configuration.resolve(Configuration.java:421)
>> at java.base/java.lang.module.Configuration.resolve(Configuration.java:255)
>> at jdk.jlink/jdk.tools.jlink.internal.JlinkTask.limitFinder(JlinkTask.java:545)
>> at jdk.jlink/jdk.tools.jlink.internal.JlinkTask.newModuleFinder(JlinkTask.java:466)
>> at jdk.jlink/jdk.tools.jlink.internal.JlinkTask.initJlinkConfig(JlinkTask.java:374)
>> at jdk.jlink/jdk.tools.jlink.internal.JlinkTask.run(JlinkTask.java:267)
>> at jdk.jlink/jdk.tools.jlink.internal.Main.run(Main.java:54)
>> at jdk.jlink/jdk.tools.jlink.internal.Main$JlinkToolProvider.run(Main.java:65)
>> at tests.JImageGenerator$JLinkTask.call(JImageGenerator.java:715)
>> at tests.Helper.generateDefaultImage(Helper.java:257)
>> at tests.Helper.generateDefaultImage(Helper.java:244)
>> at JLinkNegativeTest.testSectionsAreFiles(JLinkNegativeTest.java:307)
>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
>> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>> at java.base/java.lang.reflect.Method.invoke(Method.java:568)
>> at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85)
>> at org.testng.internal.Invoker.invokeMethod(Invoker.java:639)
>> at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821)
>> at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1131)
>> at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:125)
>> at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108)
>> at org.testng.TestRunner.privateRun(TestRunner.java:773)
>> at org.testng.TestRunner.run(TestRunner.java:623)
>> at org.testng.SuiteRunner.runTest(SuiteRunner.java:357)
>> at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352)
>> at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310)
>> at org.testng.SuiteRunner.run(SuiteRunner.java:259)
>> at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
>> at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
>> at org.testng.TestNG.runSuitesSequentially(TestNG.java:1185)
>> at org.testng.TestNG.runSuitesLocally(TestNG.java:1110)
>> at org.testng.TestNG.run(TestNG.java:1018)
>> at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94)
>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
>> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>> at java.base/java.lang.reflect.Method.invoke(Method.java:568)
>> at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298)
>> at java.base/java.lang.Thread.run(Thread.java:831)
>> Caused by: java.io.IOException: Invalid JMOD file: /home/s.tsypanov/IdeaProjects/jdk-github/build/linux-x86_64-server-release/test-support/jtreg_test_jdk_tier2/scratch/3/jmods/hacked4.jmod
>> at java.base/jdk.internal.jmod.JmodFile.checkMagic(JmodFile.java:62)
>> at java.base/jdk.internal.jmod.JmodFile.<init>(JmodFile.java:180)
>> at java.base/jdk.internal.module.ModulePath.readJMod(ModulePath.java:392)
>> at java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:343)
>> at java.base/jdk.internal.module.ModulePath.scanDirectory(ModulePath.java:284)
>> at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:232)
>> ... 44 more
>>
>> java.lang.Exception: failures: 1
>> at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:96)
>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
>> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>> at java.base/java.lang.reflect.Method.invoke(Method.java:568)
>> at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298)
>> at java.base/java.lang.Thread.run(Thread.java:831)
>>
>> So if you don't mind, I'll revert the latest commit
>
> JLinkNegativeTest depends on the exception message, it should be easy to update.

Done

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

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

[Sergey Bylokhov-2]

On Sun, 14 Mar 2021 19:35:25 GMT, Сергей Цыпанов <[hidden email]> wrote:

>> In some cases wrapping of `InputStream` with `BufferedInputStream` is redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does not require any buffer having one within.
>>
>> Other cases are related to reading either a byte or short `byte[]`: in both cases `BufferedInputStream.fill()` will be called resulting in load of much bigger amount of data (8192 by default) than required.
>
> Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision:
>
>   Use InputStream.readNBytes() and fix JLinkNegativeTest

src/java.desktop/share/classes/sun/awt/image/ByteArrayImageSource.java line 54:

> 52:
> 53:     protected ImageDecoder getDecoder() {
> 54:         InputStream is = new ByteArrayInputStream(imagedata, imageoffset, imagelength);

This file use 80 chars per line code style.

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

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

[Сергей Цыпанов-2]

On Mon, 15 Mar 2021 11:41:07 GMT, Alan Bateman <[hidden email]> wrote:

>> Done
>
>> Done
>
> I think I'd prefer if the exception message would say that the JMOD is invalid or that the JMOD file is truncated (because I don't think anyone seeing this exception will know anything about the header).

Fixed

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

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

[Сергей Цыпанов-2]

> In some cases wrapping of `InputStream` with `BufferedInputStream` is redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does not require any buffer having one within.
>
> Other cases are related to reading either a byte or short `byte[]`: in both cases `BufferedInputStream.fill()` will be called resulting in load of much bigger amount of data (8192 by default) than required.

Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision:

  Revert HttpClient

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2992/files
  - new: https://git.openjdk.java.net/jdk/pull/2992/files/ff25cc4d..af4fcce4

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2992&range=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2992&range=04-05

  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2992.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2992/head:pull/2992

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