Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v4]
On Sat, 9 Jan 2021 23:06:22 GMT, Philippe Marschall wrote: >> Implement three optimiztations for Reader.read(CharBuffer) >> >> * Add a code path for heap buffers in Reader#read to use the backing array >> instead of allocating a new one. >> * Change the code path for direct buffers in Reader#read to limit the >> intermediate allocation to `TRANSFER_BUFFER_SIZE`. >> * Implement `InputStreamReader#read(CharBuffer)` and delegate to >> `StreamDecoder`. >> * Implement `StreamDecoder#read(CharBuffer)` and avoid buffer allocation. > > Philippe Marschall has updated the pull request incrementally with one > additional commit since the last revision: > > Add unit tests > > - add unit test for Reader#read(CharBuffer) > - add unit test for InputStreamReader#reader(CharBuffer) > - test with both on-heap and off-heap buffers src/java.base/share/classes/java/io/Reader.java line 207: > 205: target.put(cbuf, 0, n); > 206: nread += n; > 207: remaining -= n; Wouldn't there be a possibility for target.put(cbuf, 0, n) to throw BufferOverflowException ? For example: - there's room (remaining) for TRANSFER_BUFFER_SIZE + 1 characters in target - cbuff is sized to TRANSFER_BUFFER_SIZE - 1st iteration of do loop transfers TRANSFER_BUFFER_SIZE charasters (remaining == 1) - 2nd iteration reads > 1 (up to TRANSFER_BUFFER_SIZE) characters - target.put throws BufferOverflowException You have to limit the amount read in each iteration to be Math.min(remaining, cbuf.length) - PR: https://git.openjdk.java.net/jdk/pull/1915
Re: RFR: 8259842: Remove Result cache from StringCoding [v7]
> The `StringCoding.resultCached` mechanism is used to remove the allocation of > a `StringCoding.Result` object on potentially hot paths used in some `String` > constructors. Using a `ThreadLocal` has overheads though, and the overhead > got a bit worse after JDK-8258596 which addresses a leak by adding a > `SoftReference`. > > This patch refactors much of the decode logic back into `String` and gets rid > of not only the `Result` cache, but the `Result` class itself along with the > `StringDecoder` class and cache. > > Microbenchmark results: > Baseline > > Benchmark (charsetName) Mode Cnt > ScoreError Units > decodeCharsetUS-ASCII avgt 15 > 193.043 ± 8.207 ns/op > decodeCharset:·gc.alloc.rate.normUS-ASCII avgt 15 > 112.009 ± 0.001B/op > decodeCharset ISO-8859-1 avgt 15 > 164.580 ± 6.514 ns/op > decodeCharset:·gc.alloc.rate.norm ISO-8859-1 avgt 15 > 112.009 ± 0.001B/op > decodeCharset UTF-8 avgt 15 > 328.370 ± 18.420 ns/op > decodeCharset:·gc.alloc.rate.norm UTF-8 avgt 15 > 224.019 ± 0.002B/op > decodeCharset MS932 avgt 15 > 328.870 ± 20.056 ns/op > decodeCharset:·gc.alloc.rate.norm MS932 avgt 15 > 232.020 ± 0.002B/op > decodeCharset ISO-8859-6 avgt 15 > 193.603 ± 12.305 ns/op > decodeCharset:·gc.alloc.rate.norm ISO-8859-6 avgt 15 > 112.010 ± 0.001B/op > decodeCharsetNameUS-ASCII avgt 15 > 209.454 ± 9.040 ns/op > decodeCharsetName:·gc.alloc.rate.normUS-ASCII avgt 15 > 112.009 ± 0.001B/op > decodeCharsetName ISO-8859-1 avgt 15 > 188.234 ± 7.533 ns/op > decodeCharsetName:·gc.alloc.rate.norm ISO-8859-1 avgt 15 > 112.009 ± 0.001B/op > decodeCharsetName UTF-8 avgt 15 > 399.463 ± 12.437 ns/op > decodeCharsetName:·gc.alloc.rate.norm UTF-8 avgt 15 > 224.019 ± 0.003B/op > decodeCharsetName MS932 avgt 15 > 358.839 ± 15.385 ns/op > decodeCharsetName:·gc.alloc.rate.norm MS932 avgt 15 > 208.017 ± 0.003B/op > decodeCharsetName ISO-8859-6 avgt 15 > 162.570 ± 7.090 ns/op > decodeCharsetName:·gc.alloc.rate.norm ISO-8859-6 avgt 15 > 112.009 ± 0.001B/op > decodeDefault N/A avgt 15 > 316.081 ± 11.101 ns/op > decodeDefault:·gc.alloc.rate.norm N/A avgt 15 > 224.019 ± 0.002B/op > > Patched: > Benchmark (charsetName) Mode Cnt > ScoreError Units > decodeCharsetUS-ASCII avgt 15 > 159.153 ± 6.082 ns/op > decodeCharset:·gc.alloc.rate.normUS-ASCII avgt 15 > 112.009 ± 0.001B/op > decodeCharset ISO-8859-1 avgt 15 > 134.433 ± 6.203 ns/op > decodeCharset:·gc.alloc.rate.norm ISO-8859-1 avgt 15 > 112.009 ± 0.001B/op > decodeCharset UTF-8 avgt 15 > 297.234 ± 21.654 ns/op > decodeCharset:·gc.alloc.rate.norm UTF-8 avgt 15 > 224.019 ± 0.002B/op > decodeCharset MS932 avgt 15 > 311.583 ± 16.445 ns/op > decodeCharset:·gc.alloc.rate.norm MS932 avgt 15 > 208.018 ± 0.002B/op > decodeCharset ISO-8859-6 avgt 15 > 156.216 ± 6.522 ns/op > decodeCharset:·gc.alloc.rate.norm ISO-8859-6 avgt 15 > 112.010 ± 0.001B/op > decodeCharsetNameUS-ASCII avgt 15 > 180.752 ± 9.411 ns/op > decodeCharsetName:·gc.alloc.rate.normUS-ASCII avgt 15 > 112.010 ± 0.001B/op > decodeCharsetName ISO-8859-1 avgt 15 > 156.170 ± 8.003 ns/op > decodeCharsetName:·gc.alloc.rate.norm ISO-8859-1 avgt 15 > 112.010 ± 0.001B/op > decodeCharsetName UTF-8 avgt 15 > 370.337 ± 22.199 ns/op > decodeCharsetName:·gc.alloc.rate.norm UTF-8 avgt 15 > 224.019 ± 0.001B/op > decodeCharsetName MS932 avgt 15 > 312.589 ±
Re: RFR: 8252412: [macos11] system dynamic libraries removed from filesystem [v2]
> 8252412: [macos11] system dynamic libraries removed from filesystem Martin Buchholz has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: JDK-8252412: [macos11] system dynamic libraries removed from filesystem - Changes: - all: https://git.openjdk.java.net/jdk/pull/2119/files - new: https://git.openjdk.java.net/jdk/pull/2119/files/578329f0..85fdffde Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2119&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2119&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2119.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2119/head:pull/2119 PR: https://git.openjdk.java.net/jdk/pull/2119
RFR: 8259911: byteArrayViewVarHandle should throw ArrayIndexOutOfBoundsException
Change `MethodHandles.byteArrayViewVarHandle` to throw `ArrayIndexOutOfBoundsException` rather than the more generic `IndexArrayOutOfBoundsException`. This feels more natural, and reduces the risk for subtle behavioral mismatch when migrating code from arrays/Unsafe to VHs. CSR: [JDK-8259912](https://bugs.openjdk.java.net/browse/JDK-8259912) - Commit messages: - byteArrayViewVarHandle should throw ArrayIndexOutOfBoundsException Changes: https://git.openjdk.java.net/jdk/pull/2124/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2124&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259911 Stats: 8 lines in 2 files changed: 5 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/2124.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2124/head:pull/2124 PR: https://git.openjdk.java.net/jdk/pull/2124
Re: RFR: 8259498: Reduce overhead of MD5 and SHA digests [v3]
> - The MD5 intrinsics added by > [JDK-8250902](https://bugs.openjdk.java.net/browse/JDK-8250902) shows that > the `int[] x` isn't actually needed. This also applies to the SHA intrinsics > from which the MD5 intrinsic takes inspiration > - Using VarHandles we can simplify the code in `ByteArrayAccess` enough to > make it acceptable to use inline and replace the array in MD5 wholesale. This > improves performance both in the presence and the absence of the intrinsic > optimization. > - Doing the exact same thing in the SHA impls would be unwieldy (64+ element > arrays), but allocating the array lazily gets most of the speed-up in the > presence of an intrinsic while being neutral in its absence. > > Baseline: > (digesterName) (length)Cnt Score > Error Units > MessageDigests.digestMD516 15 > 2714.307 ± 21.133 ops/ms > MessageDigests.digestMD5 1024 15 > 318.087 ±0.637 ops/ms > MessageDigests.digest SHA-116 15 > 1387.266 ± 40.932 ops/ms > MessageDigests.digest SHA-1 1024 15 > 109.273 ±0.149 ops/ms > MessageDigests.digestSHA-25616 15 > 995.566 ± 21.186 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.104 ±0.079 ops/ms > MessageDigests.digestSHA-51216 15 > 803.030 ± 15.722 ops/ms > MessageDigests.digestSHA-512 1024 15 > 115.611 ±0.234 ops/ms > MessageDigests.getAndDigest MD516 15 > 2190.367 ± 97.037 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 302.903 ±1.809 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1262.656 ± 43.751 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 104.889 ±3.554 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 914.541 ± 55.621 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 85.708 ±1.394 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 737.719 ± 53.671 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.307 ±1.950 ops/ms > > GC: > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 312.011 ±0.005B/op > MessageDigests.getAndDigest:·gc.alloc.rate.normSHA-116 15 > 584.020 ±0.006B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-25616 15 > 544.019 ±0.016B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-51216 15 > 1056.037 ±0.003B/op > > Target: > Benchmark (digesterName) (length)Cnt > Score Error Units > MessageDigests.digestMD516 15 > 3134.462 ± 43.685 ops/ms > MessageDigests.digestMD5 1024 15 > 323.667 ±0.633 ops/ms > MessageDigests.digest SHA-116 15 > 1418.742 ± 38.223 ops/ms > MessageDigests.digest SHA-1 1024 15 > 110.178 ±0.788 ops/ms > MessageDigests.digestSHA-25616 15 > 1037.949 ± 21.214 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.671 ±0.228 ops/ms > MessageDigests.digestSHA-51216 15 > 812.028 ± 39.489 ops/ms > MessageDigests.digestSHA-512 1024 15 > 116.738 ±0.249 ops/ms > MessageDigests.getAndDigest MD516 15 > 2314.379 ± 229.294 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 307.835 ±5.730 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1326.887 ± 63.263 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 106.611 ±2.292 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 961.589 ± 82.052 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 88.646 ±0.194 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 775.417 ± 56.775 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.904 ±2.014 ops/ms > > GC > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 232.009 ±0.006B/op > MessageDigests.getAndDigest:·gc.alloc.r
Re: RFR: 8257733: Move module-specific data from make to respective module
On 2021-01-15 19:27, mark.reinh...@oracle.com wrote: Feature JEPs are living documents until such time as they are delivered. In this case it would not be appropriate to update JEP 201, which is as much about the transition from the old source-code layout as it is about the new layout as of 2014. At this point, and given that we’d already gone beyond JEP 201 prior to this change (with `man` and `lib` subdirectories), what’d make the most sense is a new informational JEP that documents the source-code layout. Informational JEPs can, within reason, be updated over time. So I take it I need to create a new informational JEP. :-) Fine, I can do that. I assume I mostly need to extract the parts of JEP 201 that describes the (then "new") layout, update wording to show that this is a description of the current layout, and add the new additions. It'll be a very short JEP, but in this case, that's probably a virtue. /Magnus
Re: RFR: 8259498: Reduce overhead of MD5 and SHA digests [v4]
> - The MD5 intrinsics added by > [JDK-8250902](https://bugs.openjdk.java.net/browse/JDK-8250902) shows that > the `int[] x` isn't actually needed. This also applies to the SHA intrinsics > from which the MD5 intrinsic takes inspiration > - Using VarHandles we can simplify the code in `ByteArrayAccess` enough to > make it acceptable to use inline and replace the array in MD5 wholesale. This > improves performance both in the presence and the absence of the intrinsic > optimization. > - Doing the exact same thing in the SHA impls would be unwieldy (64+ element > arrays), but allocating the array lazily gets most of the speed-up in the > presence of an intrinsic while being neutral in its absence. > > Baseline: > (digesterName) (length)Cnt Score > Error Units > MessageDigests.digestMD516 15 > 2714.307 ± 21.133 ops/ms > MessageDigests.digestMD5 1024 15 > 318.087 ±0.637 ops/ms > MessageDigests.digest SHA-116 15 > 1387.266 ± 40.932 ops/ms > MessageDigests.digest SHA-1 1024 15 > 109.273 ±0.149 ops/ms > MessageDigests.digestSHA-25616 15 > 995.566 ± 21.186 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.104 ±0.079 ops/ms > MessageDigests.digestSHA-51216 15 > 803.030 ± 15.722 ops/ms > MessageDigests.digestSHA-512 1024 15 > 115.611 ±0.234 ops/ms > MessageDigests.getAndDigest MD516 15 > 2190.367 ± 97.037 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 302.903 ±1.809 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1262.656 ± 43.751 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 104.889 ±3.554 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 914.541 ± 55.621 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 85.708 ±1.394 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 737.719 ± 53.671 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.307 ±1.950 ops/ms > > GC: > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 312.011 ±0.005B/op > MessageDigests.getAndDigest:·gc.alloc.rate.normSHA-116 15 > 584.020 ±0.006B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-25616 15 > 544.019 ±0.016B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-51216 15 > 1056.037 ±0.003B/op > > Target: > Benchmark (digesterName) (length)Cnt > Score Error Units > MessageDigests.digestMD516 15 > 3134.462 ± 43.685 ops/ms > MessageDigests.digestMD5 1024 15 > 323.667 ±0.633 ops/ms > MessageDigests.digest SHA-116 15 > 1418.742 ± 38.223 ops/ms > MessageDigests.digest SHA-1 1024 15 > 110.178 ±0.788 ops/ms > MessageDigests.digestSHA-25616 15 > 1037.949 ± 21.214 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.671 ±0.228 ops/ms > MessageDigests.digestSHA-51216 15 > 812.028 ± 39.489 ops/ms > MessageDigests.digestSHA-512 1024 15 > 116.738 ±0.249 ops/ms > MessageDigests.getAndDigest MD516 15 > 2314.379 ± 229.294 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 307.835 ±5.730 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1326.887 ± 63.263 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 106.611 ±2.292 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 961.589 ± 82.052 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 88.646 ±0.194 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 775.417 ± 56.775 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.904 ±2.014 ops/ms > > GC > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 232.009 ±0.006B/op > MessageDigests.getAndDigest:·gc.alloc.r
RFR: 8259925: [Vector API] Unreasonable IndexOutOfBoundsException message when length < vlen
Hi all,
For this reproducer:
import jdk.incubator.vector.ByteVector;
import jdk.incubator.vector.VectorSpecies;
public class Test {
static final VectorSpecies SPECIES_128 = ByteVector.SPECIES_128;
static byte[] a = new byte[8];
static byte[] b = new byte[8];
public static void main(String[] args) {
ByteVector av = ByteVector.fromArray(SPECIES_128, a, 0);
av.intoArray(b, 0);
System.out.println("b: " + b[0]);
}
}
The following IndexOutOfBoundsException message (length -7) is unreasonable.
Exception in thread "main" java.lang.IndexOutOfBoundsException: Index 0 out of
bounds for length -7
It might be better to fix it like this.
Exception in thread "main" java.lang.IndexOutOfBoundsException: Index 0 out of
bounds for length 0
Thanks.
Best regards,
Jie
-
Commit messages:
- 8259925: [Vector API] Unreasonable IndexOutOfBoundsException message when
length < vlen
Changes: https://git.openjdk.java.net/jdk/pull/2127/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2127&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8259925
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/2127.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/2127/head:pull/2127
PR: https://git.openjdk.java.net/jdk/pull/2127
Re: RFR: 8257733: Move module-specific data from make to respective module [v5]
> A lot (but not all) of the data in make/data is tied to a specific module. > For instance, the publicsuffixlist is used by java.base, and fontconfig by > java.desktop. (A few directories, like mainmanifest, is *actually* used by > make for the whole build.) > > These data files should move to the module they belong to. The are, after > all, "source code" for that module that is "compiler" into resulting > deliverables, for that module. (But the "source code" language is not Java or > C, but typically a highly domain specific language or data format, and the > "compilation" is, often, a specialized transformation.) > > This misplacement of the data directory is most visible at code review time. > When such data is changed, most of the time build-dev (or the new build > label) is involved, even though this has nothing to do with the build. While > this is annoying, a worse problem is if the actual team that needs to review > the patch (i.e., the team owning the module) is missed in the review. > > ### Modules reviewed > > - [x] java.base > - [x] java.desktop > - [x] jdk.compiler > - [x] java.se Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Move characterdata templates to share/classes/java/lang. - Changes: - all: https://git.openjdk.java.net/jdk/pull/1611/files - new: https://git.openjdk.java.net/jdk/pull/1611/files/68b252b5..c4894348 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1611&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1611&range=03-04 Stats: 4 lines in 9 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/1611.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1611/head:pull/1611 PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module [v4]
On Fri, 15 Jan 2021 14:58:14 GMT, Alan Bateman wrote: >> This PR is not stale; it's just still waiting for input from @AlanBateman. > > @magicus Can the CharacterDataXXX.template files move to > src/java.base/share/classes/java/lang? @AlanBateman When I moved the charset templates, I found this gold nugget: # Copy two Java files that need no preprocessing. $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/%.java: $(CHARACTERDATA_TEMPLATES)/%.java.template $(call LogInfo, Generating $(@F)) $(call install-file) GENSRC_CHARACTERDATA += $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/CharacterDataUndefined.java \ $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/CharacterDataPrivateUse.java What this means is that we have two "template" files that are just compiled as java files, but only after being copied to gensrc. :-) That definitely needs cleaning up, but this PR is large enough as it is, so I will not do it now. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8132984: incorrect type for Reference.discovered [v2]
> Please review this change which fixes the type of the private > Reference.discovered field. It was Reference, but that's wrong because > it can be any Reference object. > > I've changed it to Reference and let that flow through, updating some > other variables that were previously somewhat incorrectly typed (usually > with an Object type parameter). The interesting change is to the > ReferenceQueue.enqueue parameter, which is now also Reference. > > This ultimately end up with a provably safe and correct, but uncheckable, > cast in ReferenceQueue.enqueue. > > An alternative might be to use a raw type for the discovered field, but I > think that ends up with more @SuppressWarnings of various flavors. I think > the unbounded wildcard approach is clearer and cleaner. > > Note that all of the pending list handling, including the discovered field, > could be moved into a non-public, non-generic, sealed(?) base class of > Reference. The pending list handling has nothing to do with the generic > parameter T. > > Testing: > mach5 tier1 and tier4 (tier4 is where vmTestbase_vm_gc_ref tests are run) Kim Barrett has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - update copyrights - Merge branch 'master' into fix_discovered_type - Use unbounded wildcard placeholders and final safe but unchecked cast - Changes: - all: https://git.openjdk.java.net/jdk/pull/1897/files - new: https://git.openjdk.java.net/jdk/pull/1897/files/ff250a19..80415b71 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1897&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1897&range=00-01 Stats: 62902 lines in 1944 files changed: 22288 ins; 24876 del; 15738 mod Patch: https://git.openjdk.java.net/jdk/pull/1897.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1897/head:pull/1897 PR: https://git.openjdk.java.net/jdk/pull/1897
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v13]
> This PR is to introduce a new random number API for the JDK. The primary API > is found in RandomGenerator and RandomGeneratorFactory. Further description > can be found in the JEP https://openjdk.java.net/jeps/356 . > > javadoc can be found at > http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html > > old PR: https://github.com/openjdk/jdk/pull/1273 Jim Laskey has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 40 commits: - Merge branch 'master' into 8248862 - Correct range used by nextBytes - Merge branch 'master' into 8248862 - Use annotation for properties. Add getDefault(). - Merge branch 'master' into 8248862 - Introduce RandomGeneratorProperties annotation - Merge branch 'master' into 8248862 - 8248862: Implement Enhanced Pseudo-Random Number Generators Added coverage testing - 8248862: Implement Enhanced Pseudo-Random Number Generators Cleanups from Chris H. - 8248862: Implement Enhanced Pseudo-Random Number Generators Propagate exception - ... and 30 more: https://git.openjdk.java.net/jdk/compare/061ffc47...772abef6 - Changes: https://git.openjdk.java.net/jdk/pull/1292/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=12 Stats: 13205 lines in 26 files changed: 11043 ins; 2046 del; 116 mod Patch: https://git.openjdk.java.net/jdk/pull/1292.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1292/head:pull/1292 PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v14]
> This PR is to introduce a new random number API for the JDK. The primary API > is found in RandomGenerator and RandomGeneratorFactory. Further description > can be found in the JEP https://openjdk.java.net/jeps/356 . > > javadoc can be found at > http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html > > old PR: https://github.com/openjdk/jdk/pull/1273 Jim Laskey has updated the pull request incrementally with one additional commit since the last revision: Update package info to credit LMX algorithm - Changes: - all: https://git.openjdk.java.net/jdk/pull/1292/files - new: https://git.openjdk.java.net/jdk/pull/1292/files/772abef6..38369702 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=12-13 Stats: 15 lines in 1 file changed: 15 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/1292.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1292/head:pull/1292 PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8132984: incorrect type for Reference.discovered
On Sat, 26 Dec 2020 03:25:51 GMT, Kim Barrett wrote:
> Please review this change which fixes the type of the private
> Reference.discovered field. It was Reference, but that's wrong because
> it can be any Reference object.
>
> I've changed it to Reference and let that flow through, updating some
> other variables that were previously somewhat incorrectly typed (usually
> with an Object type parameter). The interesting change is to the
> ReferenceQueue.enqueue parameter, which is now also Reference.
>
> This ultimately end up with a provably safe and correct, but uncheckable,
> cast in ReferenceQueue.enqueue.
>
> An alternative might be to use a raw type for the discovered field, but I
> think that ends up with more @SuppressWarnings of various flavors. I think
> the unbounded wildcard approach is clearer and cleaner.
>
> Note that all of the pending list handling, including the discovered field,
> could be moved into a non-public, non-generic, sealed(?) base class of
> Reference. The pending list handling has nothing to do with the generic
> parameter T.
>
> Testing:
> mach5 tier1 and tier4 (tier4 is where vmTestbase_vm_gc_ref tests are run)
Hi Kim,
If you introduce a private method in Reference:
private void enqueueFromPending() {
var q = queue;
if (q != ReferenceQueue.NULL) q.enqueue(this);
}
...and use it Reference.processPendingReferences while loop like this:
if (ref instanceof Cleaner) {
...
} else {
ref.enqueueFromPending();
}
Then you can keep the signature of `ReferenceQueue.enqueue(Reference r)` and no unchecked casts are needed there.
But what you have is OK and much better than what was before.
-
PR: https://git.openjdk.java.net/jdk/pull/1897
Re: RFR: 8259842: Remove Result cache from StringCoding [v7]
On Mon, 18 Jan 2021 09:26:07 GMT, Claes Redestad wrote: >> The `StringCoding.resultCached` mechanism is used to remove the allocation >> of a `StringCoding.Result` object on potentially hot paths used in some >> `String` constructors. Using a `ThreadLocal` has overheads though, and the >> overhead got a bit worse after JDK-8258596 which addresses a leak by adding >> a `SoftReference`. >> >> This patch refactors much of the decode logic back into `String` and gets >> rid of not only the `Result` cache, but the `Result` class itself along with >> the `StringDecoder` class and cache. >> >> Microbenchmark results: >> Baseline >> >> Benchmark (charsetName) Mode Cnt >>ScoreError Units >> decodeCharsetUS-ASCII avgt 15 >> 193.043 ± 8.207 ns/op >> decodeCharset:·gc.alloc.rate.normUS-ASCII avgt 15 >> 112.009 ± 0.001B/op >> decodeCharset ISO-8859-1 avgt 15 >> 164.580 ± 6.514 ns/op >> decodeCharset:·gc.alloc.rate.norm ISO-8859-1 avgt 15 >> 112.009 ± 0.001B/op >> decodeCharset UTF-8 avgt 15 >> 328.370 ± 18.420 ns/op >> decodeCharset:·gc.alloc.rate.norm UTF-8 avgt 15 >> 224.019 ± 0.002B/op >> decodeCharset MS932 avgt 15 >> 328.870 ± 20.056 ns/op >> decodeCharset:·gc.alloc.rate.norm MS932 avgt 15 >> 232.020 ± 0.002B/op >> decodeCharset ISO-8859-6 avgt 15 >> 193.603 ± 12.305 ns/op >> decodeCharset:·gc.alloc.rate.norm ISO-8859-6 avgt 15 >> 112.010 ± 0.001B/op >> decodeCharsetNameUS-ASCII avgt 15 >> 209.454 ± 9.040 ns/op >> decodeCharsetName:·gc.alloc.rate.normUS-ASCII avgt 15 >> 112.009 ± 0.001B/op >> decodeCharsetName ISO-8859-1 avgt 15 >> 188.234 ± 7.533 ns/op >> decodeCharsetName:·gc.alloc.rate.norm ISO-8859-1 avgt 15 >> 112.009 ± 0.001B/op >> decodeCharsetName UTF-8 avgt 15 >> 399.463 ± 12.437 ns/op >> decodeCharsetName:·gc.alloc.rate.norm UTF-8 avgt 15 >> 224.019 ± 0.003B/op >> decodeCharsetName MS932 avgt 15 >> 358.839 ± 15.385 ns/op >> decodeCharsetName:·gc.alloc.rate.norm MS932 avgt 15 >> 208.017 ± 0.003B/op >> decodeCharsetName ISO-8859-6 avgt 15 >> 162.570 ± 7.090 ns/op >> decodeCharsetName:·gc.alloc.rate.norm ISO-8859-6 avgt 15 >> 112.009 ± 0.001B/op >> decodeDefault N/A avgt 15 >> 316.081 ± 11.101 ns/op >> decodeDefault:·gc.alloc.rate.norm N/A avgt 15 >> 224.019 ± 0.002B/op >> >> Patched: >> Benchmark (charsetName) Mode Cnt >>ScoreError Units >> decodeCharsetUS-ASCII avgt 15 >> 159.153 ± 6.082 ns/op >> decodeCharset:·gc.alloc.rate.normUS-ASCII avgt 15 >> 112.009 ± 0.001B/op >> decodeCharset ISO-8859-1 avgt 15 >> 134.433 ± 6.203 ns/op >> decodeCharset:·gc.alloc.rate.norm ISO-8859-1 avgt 15 >> 112.009 ± 0.001B/op >> decodeCharset UTF-8 avgt 15 >> 297.234 ± 21.654 ns/op >> decodeCharset:·gc.alloc.rate.norm UTF-8 avgt 15 >> 224.019 ± 0.002B/op >> decodeCharset MS932 avgt 15 >> 311.583 ± 16.445 ns/op >> decodeCharset:·gc.alloc.rate.norm MS932 avgt 15 >> 208.018 ± 0.002B/op >> decodeCharset ISO-8859-6 avgt 15 >> 156.216 ± 6.522 ns/op >> decodeCharset:·gc.alloc.rate.norm ISO-8859-6 avgt 15 >> 112.010 ± 0.001B/op >> decodeCharsetNameUS-ASCII avgt 15 >> 180.752 ± 9.411 ns/op >> decodeCharsetName:·gc.alloc.rate.normUS-ASCII avgt 15 >> 112.010 ± 0.001B/op >> decodeCharsetName ISO-8859-1 avgt 15 >> 156.170 ± 8.003 ns/op >> decodeCharsetName:·gc.alloc.rate.norm ISO-8859-1 avgt 15 >> 112.010 ± 0.001B/op >> decodeCharsetName UTF-8 avgt 15 >> 370.337 ± 22.199 ns/op >> decodeCharsetName:·gc.alloc.rate.norm
Performance regression in BuiltinClassLoader?
Hello, I've been looking into ZipFile again with the aim to speed up typical enterprise classloading / resource lookups. To test the performance impact of my changes, I made a benchmark which creates an EnterpriseClassLoader (subclass of URLClassLoader) with the jar files in Spring Petclinic (89 jars with ~30k entries total). (This is intentionally implemented using a plain main method with no JMH or warmup since I want to measure a representative cold application startup) The benchmark measures the time needed to call getResource 30K times, once for each entry name. Using 15.0.1, this completes in ~1700 ms Using 17 master, it completes in 12851 ms If I run the benchmark from the classpath instead of as a module, the result is even worse: 15: 1724 ms 17: 21885 ms If also measure GC collections and times, and get the following: 15: Collections: 4 Collection time: 12 ms 17: Collections: 24 Collection time: 44 ms (However, a JHM with -prof gc indicates that both 17 and 15 allocate ~5.6Kb/getResource after warmup, so not convinced this needs to be allocation related) The EnterpriseClassLoader is a bit clever in that it can perform lookups using different strategies: Regular parent-first, self-only, or use the bootstrap / platform system classloaders directly. Here are the results on 17 using each strategy (benchmark on class path): PARENT_DELEGATION: 23111 ms SELF_ONLY: 428 ms BOOTSTRAP: 8131 ms PLATFORM: 15149 ms SYSTEM: 20628 ms Here are the same results on 15: PARENT_DELEGATION: 1691 ms SELF_ONLY: 393 ms BOOTSTRAP: 579 ms PLATFORM: 908 ms SYSTEM: 1368 ms Note that the delegation chain is Enterprise -> System -> Platform -> Bootstrap. Interesting to note that URLClassLoader itself does not seem to regress much when only looking up its own resources, while delegating up the chain does. Has there been any significant work in class loading / resource lookup since 15.0.1 that could help explain this regression? Eirik.
RFR: 8259942: Enable customizations in CompileJavaModules.gmk and Main.gmk
Ensure make files look on the include path or in PHASE_MAKEDIRS for customizations. Change also adds a tidy-up that improves readability. - Commit messages: - 8259942: Enable customizations in CompileJavaModules.gmk and Main.gmk Changes: https://git.openjdk.java.net/jdk/pull/2134/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2134&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259942 Stats: 5 lines in 3 files changed: 1 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2134.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2134/head:pull/2134 PR: https://git.openjdk.java.net/jdk/pull/2134
Re: Performance regression in BuiltinClassLoader?
For reference, here's the EnterpriseClassloader benchmark I made.
It assumes you have the jar files from Spring Petclinic in BOOT-INF/lib
(but any large collection of jars should work).
public class ZIPBenchmark {
public static void main(String[] args) {
final File libDir = new File("BOOT-INF/lib/");
final Strategy strategy = Strategy.valueOf(args[0]);
EnterpriseClassLoader classLoader =
EnterpriseClassLoader.create(libDir, strategy);
final List names = classLoader.getNames();
long gcTimeBefore = 0;
long gcCountBefore = 0;
for (GarbageCollectorMXBean bean :
ManagementFactory.getGarbageCollectorMXBeans()) {
gcCountBefore += bean.getCollectionCount();
gcTimeBefore += bean.getCollectionTime();
}
long before = System.nanoTime();
for (String name : names) {
classLoader.getResource(name);
}
long gcTimeAfter = 0;
long gcCountAfter = 0;
for (GarbageCollectorMXBean bean :
ManagementFactory.getGarbageCollectorMXBeans()) {
gcCountAfter += bean.getCollectionCount();
gcTimeAfter += bean.getCollectionTime();
}
final long time = System.nanoTime() - before;
System.out.printf("%s: took %s ms\n", strategy,
TimeUnit.NANOSECONDS.toMillis(time));
System.out.printf("Collections: %s\n", gcCountAfter-gcCountBefore);
System.out.printf("Collection time: %s ms\n",
gcTimeAfter-gcTimeBefore);
}
public enum Strategy {
SELF_ONLY, BOOTSTRAP, PLATFORM, SYSTEM, PARENT_DELEGATION
}
private static class EnterpriseClassLoader extends URLClassLoader {
private final List names;
private final Strategy lookupStrategy;
public EnterpriseClassLoader(URL[] urls, List names,
Strategy strategy) {
super(urls, strategy == Strategy.BOOTSTRAP ? null :
ClassLoader.getSystemClassLoader());
this.names = names;
this.lookupStrategy = strategy;
}
public static EnterpriseClassLoader create(File libDir, Strategy
strategy) {
final File[] jarFiles = libDir.listFiles(f ->
f.getName().endsWith(".jar"));
final URL[] urls = Stream.of(jarFiles)
.map(File::toURI).map(EnterpriseClassLoader::toURL)
.toArray(URL[]::new);
List names = Stream.of(jarFiles)
.flatMap(EnterpriseClassLoader::entryNameStream)
.collect(Collectors.toList());
return new EnterpriseClassLoader(urls, names, strategy);
}
@Override
public URL getResource(String name) {
if(lookupStrategy == Strategy.BOOTSTRAP) {
return super.getResource(name);
} else if (lookupStrategy == Strategy.PLATFORM) {
return
ClassLoader.getPlatformClassLoader().getResource(name);
} else if (lookupStrategy == Strategy.SYSTEM) {
return ClassLoader.getSystemClassLoader().getResource(name);
} else if(lookupStrategy == Strategy.PARENT_DELEGATION) {
return super.getResource(name);
} else if(lookupStrategy == Strategy.SELF_ONLY) {
return super.findResource(name);
} else {
throw new IllegalStateException("Unknown strategy");
}
}
@Override
public URL findResource(String name) {
if(lookupStrategy == Strategy.BOOTSTRAP) {
return null;
} else {
return super.findResource(name);
}
}
private static Stream entryNameStream(File url) {
List names;
try (JarFile file = new JarFile(url)) {
names = file.stream()
.map(ZipEntry::getName)
.filter(name -> name.endsWith(".class"))
.collect(Collectors.toList());
} catch (IOException e) {
throw new RuntimeException(e);
}
return names.stream();
}
private static URL toURL(URI uri) {
try {
return uri.toURL();
} catch (MalformedURLException e) {
throw new RuntimeException(e);
}
}
public List getNames() {
return names;
}
}
}
On Mon, Jan 18, 2021 at 7:34 PM Eirik Bjørsnøs wrote:
>
> Hello,
>
> I've been looking into ZipFile again with the aim to speed up typical
> enterprise classloading / resource lookups.
>
> To test the performance impact of my changes, I made a benchmark which
> creates an EnterpriseClassLoader (subclass of URLClassLoader) with the jar
> files in Spring Petclinic (89 jars with ~30k entries total).
>
> (This is intentionally implemented using a plain main method with no JMH
> or warmup since I want to measure a representative cold ap
Re: Performance regression in BuiltinClassLoader?
I also measured the actual startup time of Spring Petclinic to assess real-life impact. Although there is some variance from run to run, it does seem to regress: 15: Typical startup time 7500 - 7800 ms 17: Typical startup time: 8100 - 8700 ms Eirik. On Mon, Jan 18, 2021 at 7:34 PM Eirik Bjørsnøs wrote: > > Hello, > > I've been looking into ZipFile again with the aim to speed up typical > enterprise classloading / resource lookups. > > To test the performance impact of my changes, I made a benchmark which > creates an EnterpriseClassLoader (subclass of URLClassLoader) with the jar > files in Spring Petclinic (89 jars with ~30k entries total). > > (This is intentionally implemented using a plain main method with no JMH > or warmup since I want to measure a representative cold application startup) > > The benchmark measures the time needed to call getResource 30K times, once > for each entry name. > > Using 15.0.1, this completes in ~1700 ms > Using 17 master, it completes in 12851 ms > > If I run the benchmark from the classpath instead of as a module, the > result is even worse: > > 15: 1724 ms > 17: 21885 ms > > If also measure GC collections and times, and get the following: > > 15: > Collections: 4 > Collection time: 12 ms > > 17: > Collections: 24 > Collection time: 44 ms > > (However, a JHM with -prof gc indicates that both 17 and 15 allocate > ~5.6Kb/getResource after warmup, so not convinced this needs to be > allocation related) > > The EnterpriseClassLoader is a bit clever in that it can perform lookups > using different strategies: Regular parent-first, self-only, or use the > bootstrap / platform system classloaders directly. > > Here are the results on 17 using each strategy (benchmark on class path): > > PARENT_DELEGATION: 23111 ms > SELF_ONLY: 428 ms > BOOTSTRAP: 8131 ms > PLATFORM: 15149 ms > SYSTEM: 20628 ms > > Here are the same results on 15: > > PARENT_DELEGATION: 1691 ms > SELF_ONLY: 393 ms > BOOTSTRAP: 579 ms > PLATFORM: 908 ms > SYSTEM: 1368 ms > > Note that the delegation chain is Enterprise -> System -> Platform -> > Bootstrap. > > Interesting to note that URLClassLoader itself does not seem to regress > much when only looking up its own resources, while delegating up the chain > does. > > Has there been any significant work in class loading / resource lookup > since 15.0.1 that could help explain this regression? > > Eirik. > > >
Re: Performance regression in BuiltinClassLoader?
For good measure, I did a JFR recording which revealed that ExplodedModuleReader was doing file stat in 263 of 277 native method samples. Which lie explains all this, since the 15 I used was not shipped with exploded jmods.. How do I build OpenJDK with packaged modules? Cheers, Eirik. On Mon, Jan 18, 2021 at 7:59 PM Eirik Bjørsnøs wrote: > > I also measured the actual startup time of Spring Petclinic to assess > real-life impact. Although there is some variance from run to run, it does > seem to regress: > > 15: Typical startup time 7500 - 7800 ms > 17: Typical startup time: 8100 - 8700 ms > > Eirik. > > On Mon, Jan 18, 2021 at 7:34 PM Eirik Bjørsnøs wrote: > >> >> Hello, >> >> I've been looking into ZipFile again with the aim to speed up typical >> enterprise classloading / resource lookups. >> >> To test the performance impact of my changes, I made a benchmark which >> creates an EnterpriseClassLoader (subclass of URLClassLoader) with the jar >> files in Spring Petclinic (89 jars with ~30k entries total). >> >> (This is intentionally implemented using a plain main method with no JMH >> or warmup since I want to measure a representative cold application startup) >> >> The benchmark measures the time needed to call getResource 30K times, >> once for each entry name. >> >> Using 15.0.1, this completes in ~1700 ms >> Using 17 master, it completes in 12851 ms >> >> If I run the benchmark from the classpath instead of as a module, the >> result is even worse: >> >> 15: 1724 ms >> 17: 21885 ms >> >> If also measure GC collections and times, and get the following: >> >> 15: >> Collections: 4 >> Collection time: 12 ms >> >> 17: >> Collections: 24 >> Collection time: 44 ms >> >> (However, a JHM with -prof gc indicates that both 17 and 15 allocate >> ~5.6Kb/getResource after warmup, so not convinced this needs to be >> allocation related) >> >> The EnterpriseClassLoader is a bit clever in that it can perform lookups >> using different strategies: Regular parent-first, self-only, or use the >> bootstrap / platform system classloaders directly. >> >> Here are the results on 17 using each strategy (benchmark on class path): >> >> PARENT_DELEGATION: 23111 ms >> SELF_ONLY: 428 ms >> BOOTSTRAP: 8131 ms >> PLATFORM: 15149 ms >> SYSTEM: 20628 ms >> >> Here are the same results on 15: >> >> PARENT_DELEGATION: 1691 ms >> SELF_ONLY: 393 ms >> BOOTSTRAP: 579 ms >> PLATFORM: 908 ms >> SYSTEM: 1368 ms >> >> Note that the delegation chain is Enterprise -> System -> Platform -> >> Bootstrap. >> >> Interesting to note that URLClassLoader itself does not seem to regress >> much when only looking up its own resources, while delegating up the chain >> does. >> >> Has there been any significant work in class loading / resource lookup >> since 15.0.1 that could help explain this regression? >> >> Eirik. >> >> >>
Re: Performance regression in BuiltinClassLoader?
On 18/01/2021 19:24, Eirik Bjørsnøs wrote: For good measure, I did a JFR recording which revealed that ExplodedModuleReader was doing file stat in 263 of 277 native method samples. Which lie explains all this, since the 15 I used was not shipped with exploded jmods.. How do I build OpenJDK with packaged modules? Have you done "make images"? You should see images/jdk in your build output. -Alan
Re: Performance regression in BuiltinClassLoader?
Alan, I have been using "make images" all along. This produces build/macosx-x86_64-server-release/jdk/modules with unpacked modules. I'm a bit confused since "make help" seems to indicate that "make jdk" should create unpacked modules, while "make images" should perhaps not? Or did I misunderstand? Eirik. On Mon, Jan 18, 2021 at 8:31 PM Alan Bateman wrote: > On 18/01/2021 19:24, Eirik Bjørsnøs wrote: > > For good measure, I did a JFR recording which revealed that > > ExplodedModuleReader was doing file stat in 263 of 277 native method > > samples. > > > > Which lie explains all this, since the 15 I used was not shipped with > > exploded jmods.. > > > > How do I build OpenJDK with packaged modules? > > > Have you done "make images"? You should see images/jdk in your build > output. > > -Alan >
Re: RFR: 8132984: incorrect type for Reference.discovered
> On Jan 18, 2021, at 12:28 PM, Peter Levart wrote:
> If you introduce a private method in Reference:
>
>private void enqueueFromPending() {
>var q = queue;
>if (q != ReferenceQueue.NULL) q.enqueue(this);
>}
>
> ...and use it Reference.processPendingReferences while loop like this:
>
>if (ref instanceof Cleaner) {
>...
>} else {
>ref.enqueueFromPending();
>}
>
> Then you can keep the signature of `ReferenceQueue.enqueue(Reference extends T> r)` and no unchecked casts are needed there.
Nice! And this reverts all changes to ReferenceQueue.java
> But what you have is OK and much better than what was before.
Thanks, but I’m going to take your improvement. I’ll update the PR once I’ve
re-run some tests.
Re: Performance regression in BuiltinClassLoader?
On 18/01/2021 19:35, Eirik Bjørsnøs wrote: Alan, I have been using "make images" all along. This produces build/macosx-x86_64-server-release/jdk/modules with unpacked modules. build/macosx-x86_64-server-release/jdk is an "exploded image". Think of it as an intermediate build or step in the build. If you are doing "make images" then you should been an images build in build/macosx-x86_64-server-release/images/jdk. -Alan.
Re: Performance regression in BuiltinClassLoader?
Alan, Apologies for wasting everyone's time (my own included, although I learned a lot!) I found images/jdk, and with that there is no regression. Back to square one :-) Thanks, Eirik. On Mon, Jan 18, 2021 at 8:35 PM Eirik Bjørsnøs wrote: > > Alan, > > I have been using "make images" all along. This > produces build/macosx-x86_64-server-release/jdk/modules with unpacked > modules. > > I'm a bit confused since "make help" seems to indicate that "make jdk" > should create unpacked modules, while "make images" should perhaps not? Or > did I misunderstand? > > Eirik. > > > > On Mon, Jan 18, 2021 at 8:31 PM Alan Bateman > wrote: > >> On 18/01/2021 19:24, Eirik Bjørsnøs wrote: >> > For good measure, I did a JFR recording which revealed that >> > ExplodedModuleReader was doing file stat in 263 of 277 native method >> > samples. >> > >> > Which lie explains all this, since the 15 I used was not shipped with >> > exploded jmods.. >> > >> > How do I build OpenJDK with packaged modules? >> > >> Have you done "make images"? You should see images/jdk in your build >> output. >> >> -Alan >> >
Re: Performance regression in BuiltinClassLoader?
No problem :-) I've been advocating for renaming the /jdk intermediary into something that would make it perfectly obvious to newcomers that _this is not it_, but I keep getting shot down. Short name convenient! /Claes On 2021-01-18 20:53, Eirik Bjørsnøs wrote: Alan, Apologies for wasting everyone's time (my own included, although I learned a lot!) I found images/jdk, and with that there is no regression. Back to square one :-) Thanks, Eirik. On Mon, Jan 18, 2021 at 8:35 PM Eirik Bjørsnøs wrote: Alan, I have been using "make images" all along. This produces build/macosx-x86_64-server-release/jdk/modules with unpacked modules. I'm a bit confused since "make help" seems to indicate that "make jdk" should create unpacked modules, while "make images" should perhaps not? Or did I misunderstand? Eirik. On Mon, Jan 18, 2021 at 8:31 PM Alan Bateman wrote: On 18/01/2021 19:24, Eirik Bjørsnøs wrote: For good measure, I did a JFR recording which revealed that ExplodedModuleReader was doing file stat in 263 of 277 native method samples. Which lie explains all this, since the 15 I used was not shipped with exploded jmods.. How do I build OpenJDK with packaged modules? Have you done "make images"? You should see images/jdk in your build output. -Alan
Re: A Bug involving MethodHandles, Nestmates, Reflection and @CallerSensitive
Hi Johannes,
There has been a couple of the prototypes regarding @CS methods (Alan
did one and I did another) in the context of JDK-6824466. There are lots
of security consideration regarding @CS methods. You are welcome to go
deeper in that area. If you are looking for starter bugs to fix, that
won't be a quick patch.
I also came up with a patch for JDK-8013527 when working on
JDK-6824466. It's buried in
https://github.com/openjdk/jdk/compare/master...mlchung:method-invoke. I
will extract that fix and post a PR on my proposed fix.
Mandy
On 1/16/21 10:07 AM, Johannes Kuhn wrote:
After digging though the JBS, I found this comment from John Rose[1].
I like the general idea, but I don't think it's necessary to use a
native method as stub. Instead it could be written like this:
class Class {
@CallerSensitive
@ForceInline
public static Class forName(String name) {
return forName(name, Reflection.getCallerClass());
}
private static Class forName(String name, Class caller) {
// original implementation
}
}
By doing this, binding to the caller could be done by returning a
MethodHandle that actually calls the private method - with the lookup
class injected as argument (MethodHandles.insertArguments).
Problem are methods that could be virtually invoked
(getContextClassLoader). Therefore it might be useful to keep the old
binding logic around. It also reduces the number of places where this
change has to be done - it's only required for the
hyper-@CallerSensitive methods.
I will try to write a prototype that demonstrates that this approach
is feasible.
- Johannes
[1]:
https://bugs.openjdk.java.net/browse/JDK-8020968?focusedCommentId=13611844&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13611844
On 09-Dec-20 21:09, Johannes Kuhn wrote:
On 09-Dec-20 19:44, Mandy Chung wrote:
On 12/8/20 6:02 PM, Johannes Kuhn wrote:
There are a lot of things to consider when trying to fix JDK-8013527.
Exactly in particular security implication! What is clear is that
the expected lookup class should not be the injected class. The
key message here is that we can't fix JDK-8257874 until we fix
JDK-8013527 unfortunately.
Mandy
Yeah, if JDK-8013527 is fixed it might fix JDK-8257874 as a byproduct.
If Lookup.lookup() can determine the original caller, then
Field.set*/Method.invoke could do the same.
Special care has to be taken that no other class could spoof such an
injected invoker.
Too complicated for me :). JDK-8013527 needs a sound design to
approach fixing it IMHO.
- Johannes
Re: RFR: 8132984: incorrect type for Reference.discovered [v3]
> Please review this change which fixes the type of the private > Reference.discovered field. It was Reference, but that's wrong because > it can be any Reference object. > > I've changed it to Reference and let that flow through, updating some > other variables that were previously somewhat incorrectly typed (usually > with an Object type parameter). The interesting change is to the > ReferenceQueue.enqueue parameter, which is now also Reference. > > This ultimately end up with a provably safe and correct, but uncheckable, > cast in ReferenceQueue.enqueue. > > An alternative might be to use a raw type for the discovered field, but I > think that ends up with more @SuppressWarnings of various flavors. I think > the unbounded wildcard approach is clearer and cleaner. > > Note that all of the pending list handling, including the discovered field, > could be moved into a non-public, non-generic, sealed(?) base class of > Reference. The pending list handling has nothing to do with the generic > parameter T. > > Testing: > mach5 tier1 and tier4 (tier4 is where vmTestbase_vm_gc_ref tests are run) Kim Barrett has updated the pull request incrementally with one additional commit since the last revision: plevart improvement - Changes: - all: https://git.openjdk.java.net/jdk/pull/1897/files - new: https://git.openjdk.java.net/jdk/pull/1897/files/80415b71..b95f5140 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1897&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1897&range=01-02 Stats: 23 lines in 2 files changed: 10 ins; 9 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/1897.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1897/head:pull/1897 PR: https://git.openjdk.java.net/jdk/pull/1897
RFR: 8259947: Optimize UnixPath.encode
This patch improves `UnixPath.encode` by reusing `JLA.getBytesNoRepl` (which has fast-paths for common encoding) and avoiding a `toCharArray` call on the input by refactoring the `normalizeNativePath` code to operate on `String`. This might have a cost on files on Mac that need additional native normalization. This removes another `ThreadLocal` and a source of `SoftReference`s. Together with the UTF-8 fast-path my UTF-8 encoded file system see substantial speed-ups in a trivial `new File(str).toPath()` microbenchmark. - Commit messages: - Add micro. To properly examine cost of toPath() needs a new File due caching - use FileOpen as a baseline - Optimize UnixPath.encode Changes: https://git.openjdk.java.net/jdk/pull/2135/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2135&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259947 Stats: 138 lines in 6 files changed: 79 ins; 45 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/2135.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2135/head:pull/2135 PR: https://git.openjdk.java.net/jdk/pull/2135
Re: RFR: 8259947: Optimize UnixPath.encode
On Tue, 19 Jan 2021 00:35:51 GMT, Claes Redestad wrote: > This patch improves `UnixPath.encode` by reusing `JLA.getBytesNoRepl` (which > has fast-paths for common encoding) and avoiding a `toCharArray` call on the > input by refactoring the `normalizeNativePath` code to operate on `String`. > This might have a cost on files on Mac that need additional native > normalization. > > This removes another `ThreadLocal` and a source of `SoftReference`s. Together > with the UTF-8 fast-path my UTF-8 encoded file system see substantial > speed-ups in a trivial `new File(str).toPath()` microbenchmark. Microbenchmark results, baseline: Benchmark Mode Cnt ScoreError Units FileToPath.mixavgt 15 1669.996 ± 76.308 ns/op FileToPath.normalized avgt 15 349.300 ± 16.851 ns/op FileToPath.notNormalized avgt 15 553.013 ± 28.736 ns/op FileToPath.trailingSlash avgt 15 415.107 ± 18.322 ns/op Target: Benchmark Mode CntScoreError Units FileToPath.mixavgt 15 887.191 ± 34.167 ns/op FileToPath.normalized avgt 15 132.653 ± 2.907 ns/op FileToPath.notNormalized avgt 15 333.678 ± 17.665 ns/op FileToPath.trailingSlash avgt 15 192.272 ± 7.644 ns/op - PR: https://git.openjdk.java.net/jdk/pull/2135
RFR: 8259867: Move encoding checks into ZipCoder
ZipFile.Source.initCEN verifies that entry names are encoding into bytes valid in the entry's encoding. It does so by calling encoding-specific checking methods, so it also needs to determine which check method to call for each entry. By moving the encoding-variant checks into ZipCoder, initCEN can instead simply call ZipCoder.checkEncoding. This makes the code easier to follow and also removes a duplication of flag checking logic found in zipCoderForPos. - Commit messages: - 8242959: Move name encoding checks to ZipCoder (cleanup) Changes: https://git.openjdk.java.net/jdk/pull/2110/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2110&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259867 Stats: 58 lines in 2 files changed: 27 ins; 28 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/2110.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2110/head:pull/2110 PR: https://git.openjdk.java.net/jdk/pull/2110
Re: [jdk16] RFR: 8259796: timed CompletableFuture.get may swallow InterruptedException [v2]
On Sun, 17 Jan 2021 18:52:04 GMT, Doug Lea wrote: >> Martin Buchholz has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains one commit: >> >> JDK-8259796 > > Marked as reviewed by dl (Reviewer). we additionally tweaked timedGet with slightly cleaner code. - PR: https://git.openjdk.java.net/jdk16/pull/126
Re: [jdk16] RFR: 8259796: timed CompletableFuture.get may swallow InterruptedException [v2]
> 8259796: timed CompletableFuture.get may swallow InterruptedException Martin Buchholz has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit: JDK-8259796 - Changes: https://git.openjdk.java.net/jdk16/pull/126/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk16&pr=126&range=01 Stats: 88 lines in 2 files changed: 29 ins; 26 del; 33 mod Patch: https://git.openjdk.java.net/jdk16/pull/126.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/126/head:pull/126 PR: https://git.openjdk.java.net/jdk16/pull/126
Re: A Bug involving MethodHandles, Nestmates, Reflection and @CallerSensitive
Thanks Mandy.
Yes, @CS is a complicated beast.
I also implemented a part of JDK-6824466 for my "proxies should use
hidden classes prototype". (Only for the "constructor for
serialization", as hidden classes can't be mentioned in the constant pool.)
For the @CS method that can be called virtually -
Thread.getContextClassLoader, I thought about those two interfaces:
interface GetCCLClassLoader {ClassLoader getContextClassLoader(); }
interface GetCCLObject {Object getContextClassLoader(); }
Insight: If a class extends Thread and implements GetCCLObject, javac
will add a bridge method - which is the caller now. Have to think more
about what this actually means.
The entire topic is very complex - but my current believe is that
JDK-6824466 is a basic building block for a lot of other work in that
area. It also has quite a few prototypes that have been developed
independently - suggesting that it is indeed a basic building block.
I did a quick look though your prototype, one question I could not
answer was "what prevents me from naming my hidden class
Something$$InjectedInvoker?".
I will try to dig deeper into that, sure. I don't think that there will
be any fully satisfying solution in the next months. And then I have to
convince people that those changes don't expose any security issues -
which will be quite hard.
But if you have some starter bugs that I could fix, let me know, might
help to get some reputation and familiarity with the entire process.
Thank you for your time listening to my ideas, I appreciate it :).
- Johannes
On 18-Jan-21 22:52, Mandy Chung wrote:
Hi Johannes,
There has been a couple of the prototypes regarding @CS methods (Alan
did one and I did another) in the context of JDK-6824466. There are lots
of security consideration regarding @CS methods. You are welcome to go
deeper in that area. If you are looking for starter bugs to fix, that
won't be a quick patch.
I also came up with a patch for JDK-8013527 when working on
JDK-6824466. It's buried in
https://github.com/openjdk/jdk/compare/master...mlchung:method-invoke. I
will extract that fix and post a PR on my proposed fix.
Mandy
On 1/16/21 10:07 AM, Johannes Kuhn wrote:
After digging though the JBS, I found this comment from John Rose[1].
I like the general idea, but I don't think it's necessary to use a
native method as stub. Instead it could be written like this:
class Class {
@CallerSensitive
@ForceInline
public static Class forName(String name) {
return forName(name, Reflection.getCallerClass());
}
private static Class forName(String name, Class caller) {
// original implementation
}
}
By doing this, binding to the caller could be done by returning a
MethodHandle that actually calls the private method - with the lookup
class injected as argument (MethodHandles.insertArguments).
Problem are methods that could be virtually invoked
(getContextClassLoader). Therefore it might be useful to keep the old
binding logic around. It also reduces the number of places where this
change has to be done - it's only required for the
hyper-@CallerSensitive methods.
I will try to write a prototype that demonstrates that this approach
is feasible.
- Johannes
[1]:
https://bugs.openjdk.java.net/browse/JDK-8020968?focusedCommentId=13611844&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13611844
On 09-Dec-20 21:09, Johannes Kuhn wrote:
On 09-Dec-20 19:44, Mandy Chung wrote:
On 12/8/20 6:02 PM, Johannes Kuhn wrote:
There are a lot of things to consider when trying to fix JDK-8013527.
Exactly in particular security implication! What is clear is that
the expected lookup class should not be the injected class. The
key message here is that we can't fix JDK-8257874 until we fix
JDK-8013527 unfortunately.
Mandy
Yeah, if JDK-8013527 is fixed it might fix JDK-8257874 as a byproduct.
If Lookup.lookup() can determine the original caller, then
Field.set*/Method.invoke could do the same.
Special care has to be taken that no other class could spoof such an
injected invoker.
Too complicated for me :). JDK-8013527 needs a sound design to
approach fixing it IMHO.
- Johannes
Re: Performance regression in BuiltinClassLoader?
Renaming that thing would make sense. It tripped me up too when I was new to OpenJDK. ..Thomas On Mon, Jan 18, 2021 at 9:07 PM Claes Redestad wrote: > No problem :-) > > I've been advocating for renaming the /jdk intermediary into > something that would make it perfectly obvious to newcomers that _this > is not it_, but I keep getting shot down. Short name convenient! > > /Claes > > On 2021-01-18 20:53, Eirik Bjørsnøs wrote: > > Alan, > > > > Apologies for wasting everyone's time (my own included, although I > learned > > a lot!) > > > > I found images/jdk, and with that there is no regression. > > > > Back to square one :-) > > > > Thanks, > > Eirik. > > > > > > On Mon, Jan 18, 2021 at 8:35 PM Eirik Bjørsnøs wrote: > > > >> > >> Alan, > >> > >> I have been using "make images" all along. This > >> produces build/macosx-x86_64-server-release/jdk/modules with unpacked > >> modules. > >> > >> I'm a bit confused since "make help" seems to indicate that "make jdk" > >> should create unpacked modules, while "make images" should perhaps not? > Or > >> did I misunderstand? > >> > >> Eirik. > >> > >> > >> > >> On Mon, Jan 18, 2021 at 8:31 PM Alan Bateman > >> wrote: > >> > >>> On 18/01/2021 19:24, Eirik Bjørsnøs wrote: > For good measure, I did a JFR recording which revealed that > ExplodedModuleReader was doing file stat in 263 of 277 native method > samples. > > Which lie explains all this, since the 15 I used was not shipped with > exploded jmods.. > > How do I build OpenJDK with packaged modules? > > >>> Have you done "make images"? You should see images/jdk in your build > >>> output. > >>> > >>> -Alan > >>> > >> >
Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v4]
On Mon, 18 Jan 2021 07:47:30 GMT, Peter Levart wrote: >> Philippe Marschall has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add unit tests >> >> - add unit test for Reader#read(CharBuffer) >> - add unit test for InputStreamReader#reader(CharBuffer) >> - test with both on-heap and off-heap buffers > > src/java.base/share/classes/java/io/Reader.java line 207: > >> 205: target.put(cbuf, 0, n); >> 206: nread += n; >> 207: remaining -= n; > > Wouldn't there be a possibility for target.put(cbuf, 0, n) to throw > BufferOverflowException ? > For example: > - there's room (remaining) for TRANSFER_BUFFER_SIZE + 1 characters in target > - cbuff is sized to TRANSFER_BUFFER_SIZE > - 1st iteration of do loop transfers TRANSFER_BUFFER_SIZE charasters > (remaining == 1) > - 2nd iteration reads > 1 (up to TRANSFER_BUFFER_SIZE) characters > - target.put throws BufferOverflowException > > You have to limit the amount read in each iteration to be Math.min(remaining, > cbuf.length) You're correct. I need to expand the unit tests. - PR: https://git.openjdk.java.net/jdk/pull/1915
