Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v6]
On Mon, 15 Mar 2021 18:04:26 GMT, Сергей Цыпанов 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: > > Revert HttpClient Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v6]
On Mon, 15 Mar 2021 18:01:20 GMT, Сергей Цыпанов 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: > > Revert HttpClient LGTM. Thanks! - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v6]
> 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=2992=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2992=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
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]
On Mon, 15 Mar 2021 15:04:49 GMT, Daniel Fuchs wrote:
>> Сергей Цыпанов has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix error message
>
> src/java.base/share/classes/sun/net/www/http/HttpClient.java line 434:
>
>> 432: int r = tmpbuf.read();
>> 433: if (r == -1) {
>> 434: logFinest("HttpClient.available(): " +
>
> There are some subtle things going on there. Using a `BufferedInputStream`
> ensures that all the bytes available on the socket will be read, up to the
> buffer capacity. Can you revert this change? I'd rather that this clean up be
> handled separately. I have logged
> https://bugs.openjdk.java.net/browse/JDK-8263599.
Sure, I'll revert this.
-
PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]
On Mon, 15 Mar 2021 12:19:19 GMT, Сергей Цыпанов
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:
>
> Fix error message
Changes requested by dfuchs (Reviewer).
src/java.base/share/classes/sun/net/www/http/HttpClient.java line 434:
> 432: int r = tmpbuf.read();
> 433: if (r == -1) {
> 434: logFinest("HttpClient.available(): " +
There are some subtle things going on there. Using a `BufferedInputStream`
ensures that all the bytes available on the socket will be read, up to the
buffer capacity. Can you revert this change? I'd rather that this clean up be
handled separately. I have logged
https://bugs.openjdk.java.net/browse/JDK-8263599.
-
PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]
On Mon, 15 Mar 2021 12:19:19 GMT, Сергей Цыпанов 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: > > Fix error message Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]
On Mon, 15 Mar 2021 11:41:07 GMT, Alan Bateman 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
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]
> 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: Fix error message - Changes: - all: https://git.openjdk.java.net/jdk/pull/2992/files - new: https://git.openjdk.java.net/jdk/pull/2992/files/f69c8ff4..ff25cc4d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2992=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2992=03-04 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 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
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v4]
On Sun, 14 Mar 2021 19:32:11 GMT, Сергей Цыпанов wrote: > 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). - PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v4]
> 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: Fix formatting - Changes: - all: https://git.openjdk.java.net/jdk/pull/2992/files - new: https://git.openjdk.java.net/jdk/pull/2992/files/a016d2ac..f69c8ff4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2992=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2992=02-03 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
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v3]
On Sun, 14 Mar 2021 22:48:18 GMT, Sergey Bylokhov wrote:
>> Сергей Цыпанов 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.
Fixed.
-
PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v3]
On Sun, 14 Mar 2021 19:35:25 GMT, Сергей Цыпанов
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
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v3]
On Sun, 14 Mar 2021 19:35:25 GMT, Сергей Цыпанов 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 2d change is fine. - Marked as reviewed by prr (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2992
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v3]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2992/files - new: https://git.openjdk.java.net/jdk/pull/2992/files/12505d05..a016d2ac Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2992=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2992=01-02 Stats: 5 lines in 2 files changed: 2 ins; 0 del; 3 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
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v3]
On Sun, 14 Mar 2021 17:40:27 GMT, Alan Bateman 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) >>
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v2]
On Sun, 14 Mar 2021 17:34:12 GMT, Сергей Цыпанов
wrote:
>> src/java.base/share/classes/jdk/internal/jmod/JmodFile.java line 58:
>>
>>> 56: byte[] magic = in.readNBytes(4);
>>> 57: if (magic.length != 4) {
>>> 58: throw new IOException("Header expected to be of length
>>> 4, but was " + magic.length);
>>
>> Thanks for the update. If magic != 4 then it means the file is not a JMOD
>> file or that it has been truncated.
>
> 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
>
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v2]
On Sun, 14 Mar 2021 11:16:13 GMT, Alan Bateman wrote:
>> Сергей Цыпанов has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Revert usage of InputStream.readNBytes()
>
> src/java.base/share/classes/jdk/internal/jmod/JmodFile.java line 58:
>
>> 56: byte[] magic = in.readNBytes(4);
>> 57: if (magic.length != 4) {
>> 58: throw new IOException("Header expected to be of length
>> 4, but was " + magic.length);
>
> Thanks for the update. If magic != 4 then it means the file is not a JMOD
> file or that it has been truncated.
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
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v2]
> 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 usage of InputStream.readNBytes() - Changes: - all: https://git.openjdk.java.net/jdk/pull/2992/files - new: https://git.openjdk.java.net/jdk/pull/2992/files/4c608737..12505d05 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2992=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2992=00-01 Stats: 4 lines in 1 file changed: 0 ins; 2 del; 2 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
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream
On Sat, 13 Mar 2021 22:29:23 GMT, Сергей Цыпанов
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.
src/java.base/share/classes/jdk/internal/jmod/JmodFile.java line 58:
> 56: byte[] magic = in.readNBytes(4);
> 57: if (magic.length != 4) {
> 58: throw new IOException("Header expected to be of length 4,
> but was " + magic.length);
Thanks for the update. If magic != 4 then it means the file is not a JMOD file
or that it has been truncated.
-
PR: https://git.openjdk.java.net/jdk/pull/2992
RFR: 8263560: Remove needless wrapping with BufferedInputStream
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=2992=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
Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream
On Sun, 14 Mar 2021 07:51:24 GMT, Alan Bateman 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
