Re: RFR: 8256427: Test com/sun/jndi/dns/ConfigTests/PortUnreachable.java does not work on AIX [v2]
On Tue, 17 Nov 2020 05:02:07 GMT, Christoph Langer wrote: >> The test com/sun/jndi/dns/ConfigTests/PortUnreachable.java is not working on >> AIX. >> >> It tests that when a DNS server is unreachable it fails quickly with a >> PortUnreachableException due to ICMP Destination Unreachable packets >> received and not having to wait for the full timeout interval. >> Unfortunately, on AIX such ICMP packets are not received, so the only >> exception cause will be the timeout. Hence, this test can't work on AIX, so >> it should not be executed there. >> >> At SAP, we had this test excluded for a long time already in our private >> exclude list for AIX. I suggest this test update to be the final solution. > > Christoph Langer has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains one commit: > > Test jdk/com/sun/jndi/dns/ConfigTests/PortUnreachable.java does not work on > AIX looks good, thanks for doing the change. - Marked as reviewed by mbaesken (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1241
Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second Preview)
Hi Vincente, On 16/11/2020 11:36 pm, Vicente Romero wrote: Please review the code for the second iteration of sealed classes. In this iteration we are: - Enhancing narrowing reference conversion to allow for stricter checking of cast conversions with respect to sealed type hierarchies. - Also local classes are not considered when determining implicitly declared permitted direct subclasses of a sealed class or sealed interface The major change here seems to be that getPermittedSubclasses() now returns actual Class objects instead of ClassDesc. My recollection from earlier discussions here was that the use of ClassDesc was very deliberate as the permitted subclasses may not actually exist and there may be security concerns with returning them! Cheers, David - - Commit messages: - 8246778: Compiler implementation for Sealed Classes (Second Preview) Changes: https://git.openjdk.java.net/jdk/pull/1227/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1227=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8246778 Stats: 589 lines in 12 files changed: 508 ins; 18 del; 63 mod Patch: https://git.openjdk.java.net/jdk/pull/1227.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1227/head:pull/1227 PR: https://git.openjdk.java.net/jdk/pull/1227
Integrated: 8256435: [TESTBUG] java/foreign/TestHandshake.java fails with direct buffer memory OOM
On Tue, 17 Nov 2020 09:07:03 GMT, Nick Gasson wrote:
> I ran this test on a machine with 224 logical CPUs and it fails with:
>
> ITERATION 3
> test TestHandshake.testHandshake("SegmentMismatchAccessor",
> TestHandshake$$Lambda$57/0x0001000e7968@37c4b344): failure
> java.lang.OutOfMemoryError: Cannot reserve 100 bytes of direct buffer
> memory (allocated: 536008192, limit: 536870912)
>
> SegmentMismatchAccessor allocates a 1MB native segment for each CPU on
> each iteration. This can quickly reach the allocation limit if there is
> no intervening GC. Explicitly close the segment after each iteration to
> release the memory.
This pull request has now been integrated.
Changeset: 26a1ec1b
Author:Nick Gasson
URL: https://git.openjdk.java.net/jdk/commit/26a1ec1b
Stats: 9 lines in 1 file changed: 8 ins; 0 del; 1 mod
8256435: [TESTBUG] java/foreign/TestHandshake.java fails with direct buffer
memory OOM
Reviewed-by: mcimadamore
-
PR: https://git.openjdk.java.net/jdk/pull/1254
Re: RFR: 8251317: Support for CLDR version 38
On Tue, 17 Nov 2020 23:19:23 GMT, Naoto Sato wrote: > Hi, > > Please review the changes for upgrading the CLDR data to version 38. The vast > majority of the changes are simply the changes in CLDR upstream, and others > are mainly test changes due to the locale data change. Looks like the generated webrev is empty, possibly due to this issue (https://bugs.openjdk.java.net/browse/SKARA-732). Here is the one manually generated: https://cr.openjdk.java.net/~naoto/cldr/webrev.00/ - PR: https://git.openjdk.java.net/jdk/pull/1279
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]
On Tue, 17 Nov 2020 22:21:18 GMT, Jim Laskey wrote:
>> 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 .
>
> 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
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
>
>Update package-info.java
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
>
>Updated RandomGeneratorFactory javadoc.
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
>
>Updated documentation for RandomGeneratorFactory.
> - Merge branch 'master' into 8248862
> - Merge branch 'master' into 8248862
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
>
>Move RandomGeneratorProperty
> - Merge branch 'master' into 8248862
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
>
>Clear up javadoc
> - 8248862; Implement Enhanced Pseudo-Random Number Generators
>
>remove RandomGeneratorProperty from API
> - ... and 30 more:
> https://git.openjdk.java.net/jdk/compare/f7517386...6fe94c68
I am unsure if the intent is also to support external libraries providing
`RandomGenerator` implementations. Currently there is an implicit contract for
properties (reflectively invoking a method returning a map with a set of
entries with known keys). Since the service provider implementation is the
`RandomGenerator` itself, rather than `RandomGeneratorFactory` it is harder
expose the meta-data with a clearer contract.
src/java.base/share/classes/java/util/Random.java line 592:
> 590:
> 591: @Override
> 592: public Spliterator.OfInt makeIntsSpliterator(long index, long fence,
> int origin, int bound) {
Unsure if this and the other two methods are intended to be public or not,
since they are at the end of the class and override methods of a module private
class. In principle there is nothing wrong with such `Spliterator` factories,
but wonder if they are really needed given the `Stream` returning methods. The
arrangement of classes makes it awkward to hide these methods.
src/java.base/share/classes/java/util/SplittableRandom.java line 171:
> 169: * RandomGenerator properties.
> 170: */
> 171: static Map getProperties() {
With records exiting preview in 16 this map of properties could i think be
represented as a record instance, with better type safety, where
`RandomSupport.RandomGeneratorProperty` enum values become typed fields
declared on the record class. Something to consider after integration perhaps?
src/java.base/share/classes/java/util/SplittableRandom.java line 211:
> 209: *
> http://zimbry.blogspot.com/2011/09/better-bit-mixing-improving-on.html
> 210: */
> 211: private static long mix64(long z) {
Usages be replaced with calls to `RandomSupport.mixStafford13`?
src/java.base/share/classes/module-info.java line 250:
> 248: exports jdk.internal.util.xml.impl to
> 249: jdk.jfr;
> 250: exports jdk.internal.util.random;
Unqualified export, should this be `to jdk.random`?
src/jdk.random/share/classes/module-info.java line 50:
> 48: */
> 49: module jdk.random {
> 50: uses java.util.random.RandomGenerator;
Are these `uses` declarations needed? `ServiceLoader` is not used by this
module, and `RandomSupport` is not a service interface.
src/jdk.random/share/classes/module-info.java line 53:
> 51: uses RandomSupport;
> 52:
> 53: exports jdk.random to
Why is this needed?
src/java.base/share/classes/java/util/random/package-info.java line 50:
> 48: * given its name.
> 49: *
> 50: * The principal supporting class is {@link RandomGenertatorFactor}.
> This
s/RandomGenertatorFactor/RandomGenertatorFactory
src/java.base/share/classes/java/util/random/package-info.java line 140:
> 138: *
> 139: * For applications with no special requirements,
> 140: * "L64X128MixRandom" has a good balance among speed, space,
The documentation assumes that the `jdk.random` module is present in the JDK
image. Perhaps we need to spit the specifics to `jdk.random`?
src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
1211:
> 1209: Udiff = -Udiff;
> 1210: U2 = U1;
> 1211: U1 -= Udiff;
Updated `U1` never used (recommend running the code through a checker e.g. use
IntelliJ)
src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
331:
> 329: }
> 330: // Finally, we need to make sure the last z words are not all
> zero.
> 331: search: {
Nice! a rarely used feature :-)
src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
1157:
> 1155: /*
> 1156:
Re: RFR: 8230501: Class data support for hidden classes [v4]
> Provide the `Lookup::defineHiddenClassWithClassData` API that allows live > objects > be shared between a hidden class and other classes. A hidden class can load > these live objects as dynamically-computed constants via this API. > > Specdiff > http://cr.openjdk.java.net/~mchung/jdk16/webrevs/8230501/specdiff/overview-summary.html > > With this class data support and hidden classes, > `sun.misc.Unsafe::defineAnonymousClass` > will be deprecated for removal. Existing libraries should replace their > calls to `sun.misc.Unsafe::defineAnonymousClass` with > `Lookup::defineHiddenClass` > or `Lookup::defineHiddenClassWithClassData`. > > This patch also updates the implementation of lambda meta factory and > `MemoryAccessVarHandleGenerator` to use class data. No performance > difference > observed in the jdk.incubator.foreign microbenchmarks. A side note: > `MemoryAccessVarHandleGenerator` is removed in the upcoming integration of > JDK-8254162 but it helps validating the class data support. > > Background > -- > > This is an enhancement following up JEP 371: Hidden Classes w.r.t. > "Constant-pool patching" in the "Risks and Assumption" section. > > A VM-anonymous class can be defined with its constant-pool entries already > resolved to concrete values. This allows critical constants to be shared > between a VM-anonymous class and the language runtime that defines it, and > between multiple VM-anonymous classes. For example, a language runtime will > often have `MethodHandle` objects in its address space that would be useful > to newly-defined VM-anonymous classes. Instead of the runtime serializing > the objects to constant-pool entries in VM-anonymous classes and then > generating bytecode in those classes to laboriously `ldc` the entries, > the runtime can simply supply `Unsafe::defineAnonymousClass` with references > to its live objects. The relevant constant-pool entries in the newly-defined > VM-anonymous class are pre-linked to those objects, improving performance > and reducing footprint. In addition, this allows VM-anonymous classes to > refer to each other: Constant-pool entries in a class file are based on names. > They thus cannot refer to nameless VM-anonymous classes. A language runtime > can, > however, easily track the live Class objects for its VM-anonymous classes and > supply them to `Unsafe::defineAnonymousClass`, thus pre-linking the new > class's > constant pool entries to other VM-anonymous classes. > > This extends the hidden classes to allow live objects to be injected > in a hidden class and loaded them via condy. > > Details > --- > > A new `Lookup::defineHiddenClassWithClassData` API takes additional > `classData` argument compared to `Lookup::defineHiddenClass`. > Class data can be method handles, lookup objects, arbitrary user objects > or collections of all of the above. > > This method behaves as if calling `Lookup::defineHiddenClass` to define > a hidden class with a private static unnamed field that is initialized > with `classData` at the first instruction of the class initializer. > > `MethodHandles::classData(Lookup lookup, String name, Class type)` > is a bootstrap method to load the class data of the given lookup's lookup > class. > The hidden class will be initialized when `classData` method is called if > the hidden class has not been initialized. > > For a class data containing more than one single element, libraries can > create their convenience method to load a single live object via condy. > We can reconsider if such a convenience method is needed in the future. > > Frameworks sometimes want to dynamically create a hidden class (HC) and add it > it the lookup class nest and have HC to carry secrets hidden from that nest. > In this case, frameworks should not to use private static finals (in the HCs > they spin) to hold secrets because a nestmate of HC may obtain access to > such a private static final and observe the framework's secret. It should use > condy. In addition, we need to differentiate if a lookup object is created > from > the original lookup class or created from teleporting e.g. `Lookup::in` > and `MethodHandles::privateLookupIn`. > > This proposes to add a new `ORIGINAL` bit that is only set if the lookup > object is created by `MethodHandles::lookup` or by bootstrap method > invocation. > The operations only apply to a Lookup object with original access are: >- create method handles for caller-sensitve methods >- obtain class data associated with the lookup class > > No change to `Lookup::hasFullPrivilegeAccess` and `Lookup::toString` which > ignores the ORIGINAL bit. > > > Compatibility Risks > --- > > `Lookup::lookupModes` includes a new `ORIGINAL` bit. Most lookup operations > ignore this original bit except creating method handles for caller-sensitive > methods > that expects the lookup from the original lookup class. Existing code > compares > the return value of
Re: RFR: 8253459: Formatter treats index, width and precision > Integer.MAX_VALUE incorrectly [v9]
On Tue, 17 Nov 2020 21:21:47 GMT, Roger Riggs wrote: >> Ian Graves has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Adding test coverage. Tweaking wording in docs. > > test/jdk/java/util/IllegalFormatException/ArgumentIndexException.java line 1: > >> 1: /* > > Typically, using the TestNG framework is preferable for new tests. > In addition to a convenient set of Asserts that check and print expected vs > actual and message > it includes patterns for testing for expected exceptions. Yes, these tests are amenable to TestNG's `assertThrows` method. That might be all you need; we generally aren't too concerned about the exact content and formatting of the exception's message. However, if you want to make additional assertions over the thrown exception, it can be obtained using `expectThrows` instead of `assertThrows`. - PR: https://git.openjdk.java.net/jdk/pull/516
Re: RFR: 8230501: Class data support for hidden classes [v3]
> Provide the `Lookup::defineHiddenClassWithClassData` API that allows live > objects > be shared between a hidden class and other classes. A hidden class can load > these live objects as dynamically-computed constants via this API. > > Specdiff > http://cr.openjdk.java.net/~mchung/jdk16/webrevs/8230501/specdiff/overview-summary.html > > With this class data support and hidden classes, > `sun.misc.Unsafe::defineAnonymousClass` > will be deprecated for removal. Existing libraries should replace their > calls to `sun.misc.Unsafe::defineAnonymousClass` with > `Lookup::defineHiddenClass` > or `Lookup::defineHiddenClassWithClassData`. > > This patch also updates the implementation of lambda meta factory and > `MemoryAccessVarHandleGenerator` to use class data. No performance > difference > observed in the jdk.incubator.foreign microbenchmarks. A side note: > `MemoryAccessVarHandleGenerator` is removed in the upcoming integration of > JDK-8254162 but it helps validating the class data support. > > Background > -- > > This is an enhancement following up JEP 371: Hidden Classes w.r.t. > "Constant-pool patching" in the "Risks and Assumption" section. > > A VM-anonymous class can be defined with its constant-pool entries already > resolved to concrete values. This allows critical constants to be shared > between a VM-anonymous class and the language runtime that defines it, and > between multiple VM-anonymous classes. For example, a language runtime will > often have `MethodHandle` objects in its address space that would be useful > to newly-defined VM-anonymous classes. Instead of the runtime serializing > the objects to constant-pool entries in VM-anonymous classes and then > generating bytecode in those classes to laboriously `ldc` the entries, > the runtime can simply supply `Unsafe::defineAnonymousClass` with references > to its live objects. The relevant constant-pool entries in the newly-defined > VM-anonymous class are pre-linked to those objects, improving performance > and reducing footprint. In addition, this allows VM-anonymous classes to > refer to each other: Constant-pool entries in a class file are based on names. > They thus cannot refer to nameless VM-anonymous classes. A language runtime > can, > however, easily track the live Class objects for its VM-anonymous classes and > supply them to `Unsafe::defineAnonymousClass`, thus pre-linking the new > class's > constant pool entries to other VM-anonymous classes. > > This extends the hidden classes to allow live objects to be injected > in a hidden class and loaded them via condy. > > Details > --- > > A new `Lookup::defineHiddenClassWithClassData` API takes additional > `classData` argument compared to `Lookup::defineHiddenClass`. > Class data can be method handles, lookup objects, arbitrary user objects > or collections of all of the above. > > This method behaves as if calling `Lookup::defineHiddenClass` to define > a hidden class with a private static unnamed field that is initialized > with `classData` at the first instruction of the class initializer. > > `MethodHandles::classData(Lookup lookup, String name, Class type)` > is a bootstrap method to load the class data of the given lookup's lookup > class. > The hidden class will be initialized when `classData` method is called if > the hidden class has not been initialized. > > For a class data containing more than one single element, libraries can > create their convenience method to load a single live object via condy. > We can reconsider if such a convenience method is needed in the future. > > Frameworks sometimes want to dynamically create a hidden class (HC) and add it > it the lookup class nest and have HC to carry secrets hidden from that nest. > In this case, frameworks should not to use private static finals (in the HCs > they spin) to hold secrets because a nestmate of HC may obtain access to > such a private static final and observe the framework's secret. It should use > condy. In addition, we need to differentiate if a lookup object is created > from > the original lookup class or created from teleporting e.g. `Lookup::in` > and `MethodHandles::privateLookupIn`. > > This proposes to add a new `ORIGINAL` bit that is only set if the lookup > object is created by `MethodHandles::lookup` or by bootstrap method > invocation. > The operations only apply to a Lookup object with original access are: >- create method handles for caller-sensitve methods >- obtain class data associated with the lookup class > > No change to `Lookup::hasFullPrivilegeAccess` and `Lookup::toString` which > ignores the ORIGINAL bit. > > > Compatibility Risks > --- > > `Lookup::lookupModes` includes a new `ORIGINAL` bit. Most lookup operations > ignore this original bit except creating method handles for caller-sensitive > methods > that expects the lookup from the original lookup class. Existing code > compares > the return value of
Re: RFR: 8256152: tests fail because of ambiguous method resolution [v2]
> Added a cast in the right place, thanks to @jonathan-gibbons. Stuart Marks has updated the pull request incrementally with one additional commit since the last revision: cast to double instead of Object - Changes: - all: https://git.openjdk.java.net/jdk/pull/1274/files - new: https://git.openjdk.java.net/jdk/pull/1274/files/4a401ed2..6875beb1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1274=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1274=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1274.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1274/head:pull/1274 PR: https://git.openjdk.java.net/jdk/pull/1274
RFR: 8251317: Support for CLDR version 38
Hi, Please review the changes for upgrading the CLDR data to version 38. The vast majority of the changes are simply the changes in CLDR upstream, and others are mainly test changes due to the locale data change. - Commit messages: - Updated the version in `cldr.md` files - Merge branch 'master' into cldr - 8251317: Support for CLDR version 38 Changes: https://git.openjdk.java.net/jdk/pull/1279/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1279=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8251317 Stats: 57338 lines in 209 files changed: 41958 ins; 4932 del; 10448 mod Patch: https://git.openjdk.java.net/jdk/pull/1279.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1279/head:pull/1279 PR: https://git.openjdk.java.net/jdk/pull/1279
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]
An honest question, why do we need so many interfaces for the different categories of RandomGenerator ? My fear is that we are encoding the state of our knowledge of the different kinds of random generators now so it will not be pretty in the future when new categories of random generator are discovered/invented. If we can take example of the past to predict the future, 20 years ago, what should have been the hierarchy at that time. Is it not reasonable to think that we will need new kinds of random generator in the future ? I wonder if it's not better to have one interface and several optional methods like we have with the collections, it means that we are loosing the possibilities to precisely type a method that only works with a precise type of generator but it will be more future proof. Rémi - Mail original - > De: "Jim Laskey" > À: "build-dev" , "core-libs-dev" > , > security-...@openjdk.java.net > Envoyé: Mardi 17 Novembre 2020 23:21:18 > Objet: Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators > [v3] >> 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 . > > 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 > - 8248862: Implement Enhanced Pseudo-Random Number Generators > > Update package-info.java > - 8248862: Implement Enhanced Pseudo-Random Number Generators > > Updated RandomGeneratorFactory javadoc. > - 8248862: Implement Enhanced Pseudo-Random Number Generators > > Updated documentation for RandomGeneratorFactory. > - Merge branch 'master' into 8248862 > - Merge branch 'master' into 8248862 > - 8248862: Implement Enhanced Pseudo-Random Number Generators > > Move RandomGeneratorProperty > - Merge branch 'master' into 8248862 > - 8248862: Implement Enhanced Pseudo-Random Number Generators > > Clear up javadoc > - 8248862; Implement Enhanced Pseudo-Random Number Generators > > remove RandomGeneratorProperty from API > - ... and 30 more: > https://git.openjdk.java.net/jdk/compare/f7517386...6fe94c68 > > - > > Changes: https://git.openjdk.java.net/jdk/pull/1273/files > Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1273=02 > Stats: 14891 lines in 31 files changed: 0 ins; 3704 del; 77 mod > Patch: https://git.openjdk.java.net/jdk/pull/1273.diff > Fetch: git fetch https://git.openjdk.java.net/jdk pull/1273/head:pull/1273 > > PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators
On Tue, 17 Nov 2020 22:12:19 GMT, Jim Laskey wrote: >> @kevinrushforth What is the recommended approach to remove the doc >> edit/commit? > > javadoc can be found at > http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html Presuming your master branch is in sync with the upstream jdk repo, something like the following: git checkout master -- doc git add doc git commit -m "restore doc files" - PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]
> 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 . 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 - 8248862: Implement Enhanced Pseudo-Random Number Generators Update package-info.java - 8248862: Implement Enhanced Pseudo-Random Number Generators Updated RandomGeneratorFactory javadoc. - 8248862: Implement Enhanced Pseudo-Random Number Generators Updated documentation for RandomGeneratorFactory. - Merge branch 'master' into 8248862 - Merge branch 'master' into 8248862 - 8248862: Implement Enhanced Pseudo-Random Number Generators Move RandomGeneratorProperty - Merge branch 'master' into 8248862 - 8248862: Implement Enhanced Pseudo-Random Number Generators Clear up javadoc - 8248862; Implement Enhanced Pseudo-Random Number Generators remove RandomGeneratorProperty from API - ... and 30 more: https://git.openjdk.java.net/jdk/compare/f7517386...6fe94c68 - Changes: https://git.openjdk.java.net/jdk/pull/1273/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1273=02 Stats: 14891 lines in 31 files changed: 0 ins; 3704 del; 77 mod Patch: https://git.openjdk.java.net/jdk/pull/1273.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1273/head:pull/1273 PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators
On Tue, 17 Nov 2020 21:59:04 GMT, Jim Laskey wrote: >>> my local branch seems to have the right sources for doc >> >> Maybe, but your branch on GitHub does not. > > @kevinrushforth What is the recommended approach to remove the doc > edit/commit? javadoc can be found at http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html - PR: https://git.openjdk.java.net/jdk/pull/1273
Re: 'Find' method for Iterable
- Mail original - > De: "Brian Goetz" > À: "Michael Kuhlmann" , "core-libs-dev" > > Envoyé: Mardi 17 Novembre 2020 20:44:17 > Objet: Re: 'Find' method for Iterable > On 9/21/2020 4:08 AM, Michael Kuhlmann wrote: >> But after thinking about it, I'm now convinced that it would be a bad >> idea. Because it extends the scope of this small, tiny Iterable >> interface to something bigger which it shouldn't be. > > This response captures the essence of the problem. You may think you > are asking for "just one more method on Iterable", but really, what you > are asking for is to turn Iterable into something it is not. Iterable > exists not to be "smallest collection", but as the interface to the > foreach-loop. Yes, we could have chosen a different design center for > Iterable (Eclipse Collections did, see RichIterable), but we didn't. > Adding this method merely sends the signal that we want to extend > Iterable to be more than it is, which opens the floodgate to requests > (and eventually demands) for more methods. > >> I can ask about Iterable#forEach - is it there only because it was there to >> begin with? Would it have been a bad idea to add one if we had streams >> already? > > While you can make an argument that forEach is excessive, the fact that > you make this argument illustrates the essential problem with this > proposal. Your argument wrt forEach is "If that makes sense, then so > does find." If you pull on that string, then this method forms the > basis of tomorrow's assumption: "You have forEach and find, it is > unreasonable to not add ". There is a good reason to have forEach, Iterable in the interface of things that can be iterated, and in Java, there are two ways to do an iteration, the internal iteration, where you ask the class to do the iteration for you using forEach and the external iterator, where you ask for an Iterator and use it to iterate outside of the class. In term of API design, it is push vs pull, using forEach, the element are is pushed to the Consumer while using an Iterator.next() allow you to pull the element. Usually, forEach() is faster but it takes a Consumer, so you are limited by the limitation of the lambda (you can not mutate the local variables of the enclosing scope). Using an Iterator is also more powerful because you can compose them. > > So, Iterable should stay simple. (The other arguments against it on > this thread, are also valid, but this is the most important one.) Yep. Rémi
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators
On Tue, 17 Nov 2020 21:10:48 GMT, Kevin Rushforth wrote: >> @erikj79 my local branch seems to have the right sources for doc > >> my local branch seems to have the right sources for doc > > Maybe, but your branch on GitHub does not. @kevinrushforth What is the recommended approach to remove the doc edit/commit? - PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8253459: Formatter treats index, width and precision > Integer.MAX_VALUE incorrectly [v9]
On Tue, 17 Nov 2020 19:58:29 GMT, Ian Graves wrote:
>> The `java.util.Formatter` format specifies support for field widths,
>> argument indexes, or precision lengths of a field that relate to the
>> variadic arguments supplied to the formatter. These numbers are specified by
>> integers, sometimes negative. For argument index, it's specified in the
>> documentation that the highest allowed argument is limited by the largest
>> possible index of an array (ie the largest possible variadic index), but for
>> the other two it's not defined. Moreover, what happens when a number field
>> in a string is too large or too small to be represented by a 32-bit integer
>> type is not defined.
>>
>> This fix adds documentation to specify what error behavior occurs during
>> these cases. Additionally it adds an additional exception type to throw when
>> an invalid argument index is observed.
>>
>> A CSR will be required for this PR.
>
> Ian Graves has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Adding test coverage. Tweaking wording in docs.
test/jdk/java/util/IllegalFormatException/ArgumentIndexException.java line 52:
> 50: }
> 51: }
> 52: throw new RuntimeException("Expected IllegalFormatException for
> zero argument index.");
The wording of errors around exceptions can be misinterpreted. Did an expected
exception occur or not?
If all you saw was the exception without the line of code, would it be
unambiguous?
src/java.base/share/classes/java/util/Formatter.java line 2802:
> 2800: // skip the trailing '$'
> 2801: index = Integer.parseInt(s, start, end - 1, 10);
> 2802: if(index <= 0) {
Add a space after 'if' please.
test/jdk/java/util/IllegalFormatException/ArgumentIndexException.java line 1:
> 1: /*
Typically, using the TestNG framework is preferable for new tests.
In addition to a convenient set of Asserts that check and print expected vs
actual and message
it includes patterns for testing for expected exceptions.
test/jdk/java/util/IllegalFormatException/ArgumentIndexException.java line 98:
> 96: }
> 97:
> 98: }
Add a newline at end-of-file.
src/java.base/share/classes/java/util/IllegalFormatArgumentIndexException.java
line 64:
> 62: public String getMessage() {
> 63: return String.format("Illegal format argument index = %d",
> getIndex());
> 64: }
The exception with a very large negative number isn't going to easy to
recognize.
Can the exception message say (for the Integer.MIN_VALUE) that the index is not
valid index.
Its probably too much to ask have an indication where in the format string the
offending index occurs.
-
PR: https://git.openjdk.java.net/jdk/pull/516
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators
On Tue, 17 Nov 2020 21:33:13 GMT, Magnus Ihse Bursie wrote: >>> my local branch seems to have the right sources for doc >> >> Maybe, but your branch on GitHub does not. > > This PR looks seriously messed up: > > JimLaskey added 30 commits on Oct 9 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators > 515a5a4 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 7c25f2d > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 642022d > @JimLaskey > Merge branch 'master' into 8248862 > b537e0f > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > ca90d01 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > ec2941a > @JimLaskey > Merge branch 'master' into 8248862 > dbe142c > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > cca6f0f > @JimLaskey > Merge branch 'master' into 8248862 > 93ee16f > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 84d4704 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > b50f2ee > @JimLaskey > Merge branch 'master' into 8248862 > b1d6abe > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 8afe7a2 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 2f99696 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 7c58819 > @JimLaskey > Merge branch 'master' into 8248862 > 655ad08 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > d5860b9 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 3ac78f8 > @JimLaskey > Merge branch 'master' into 8248862 > e3b6e64 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 8ec7f87 > @JimLaskey > Merge branch 'master' into 8248862 > ed959f8 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 72195e8 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 84730c3 > @JimLaskey > Merge branch 'master' into 8248862 > 05f0477 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 7dd81ac > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 38f534b > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > f428b61 > @JimLaskey > Merge branch 'master' into 8248862 > 5e33872 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 0bc8e3d > @JimLaskey > Merge branch 'master' into 8248862 > 679f1b5 > JimLaskey added 7 commits 4 days ago > @JimLaskey > 8248862; Implement Enhanced Pseudo-Random Number Generators … > 17972a9 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 4f0338f > @JimLaskey > Merge branch 'master' into 8248862 > 7c6b9fa > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > c67c729 > @JimLaskey > Merge branch 'master' into 8248862 > 108ea50 > @JimLaskey > Merge branch 'master' into 8248862 > 1b31205 > @JimLaskey > 8248862: Implement Enhanced Pseudo-Random Number Generators … > 7469ca5 > > It might be better if you close this, rebase your actual change on top of a > current master, and open a new PR instead. > > The only reason this was flagged with the `build` label was due to the > incorrect deletion of doc, so if you intend to go on with this PR, please > remove the label after you've fixed the doc issue. Ah, sorry, I see now that I expanded the commit comments that it was not just the same commit applied over and over again, as I assumed, but iterative steps from your branch while developing, that just had the same first line... sorry for the confusion about that. - PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators
On Tue, 17 Nov 2020 21:10:48 GMT, Kevin Rushforth wrote: >> @erikj79 my local branch seems to have the right sources for doc > >> my local branch seems to have the right sources for doc > > Maybe, but your branch on GitHub does not. This PR looks seriously messed up: JimLaskey added 30 commits on Oct 9 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators 515a5a4 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 7c25f2d @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 642022d @JimLaskey Merge branch 'master' into 8248862 b537e0f @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … ca90d01 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … ec2941a @JimLaskey Merge branch 'master' into 8248862 dbe142c @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … cca6f0f @JimLaskey Merge branch 'master' into 8248862 93ee16f @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 84d4704 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … b50f2ee @JimLaskey Merge branch 'master' into 8248862 b1d6abe @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 8afe7a2 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 2f99696 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 7c58819 @JimLaskey Merge branch 'master' into 8248862 655ad08 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … d5860b9 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 3ac78f8 @JimLaskey Merge branch 'master' into 8248862 e3b6e64 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 8ec7f87 @JimLaskey Merge branch 'master' into 8248862 ed959f8 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 72195e8 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 84730c3 @JimLaskey Merge branch 'master' into 8248862 05f0477 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 7dd81ac @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 38f534b @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … f428b61 @JimLaskey Merge branch 'master' into 8248862 5e33872 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 0bc8e3d @JimLaskey Merge branch 'master' into 8248862 679f1b5 JimLaskey added 7 commits 4 days ago @JimLaskey 8248862; Implement Enhanced Pseudo-Random Number Generators … 17972a9 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 4f0338f @JimLaskey Merge branch 'master' into 8248862 7c6b9fa @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … c67c729 @JimLaskey Merge branch 'master' into 8248862 108ea50 @JimLaskey Merge branch 'master' into 8248862 1b31205 @JimLaskey 8248862: Implement Enhanced Pseudo-Random Number Generators … 7469ca5 It might be better if you close this, rebase your actual change on top of a current master, and open a new PR instead. The only reason this was flagged with the `build` label was due to the incorrect deletion of doc, so if you intend to go on with this PR, please remove the label after you've fixed the doc issue. - PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators
On Tue, 17 Nov 2020 20:56:52 GMT, Jim Laskey wrote: > my local branch seems to have the right sources for doc Maybe, but your branch on GitHub does not. - PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v2]
> 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 . Jim Laskey has updated the pull request incrementally with one additional commit since the last revision: 8248862: Implement Enhanced Pseudo-Random Number Generators Updated RandomGeneratorFactory javadoc. - Changes: - all: https://git.openjdk.java.net/jdk/pull/1273/files - new: https://git.openjdk.java.net/jdk/pull/1273/files/7469ca58..b24024bb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1273=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1273=00-01 Stats: 39 lines in 6 files changed: 16 ins; 0 del; 23 mod Patch: https://git.openjdk.java.net/jdk/pull/1273.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1273/head:pull/1273 PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators
On Tue, 17 Nov 2020 20:15:34 GMT, Erik Joelsson wrote: >> 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 . > > It looks like you have deleted the doc directory. That can't be right? @erikj79 my local branch seems to have the right sources for doc @erikj79 https://github.com/JimLaskey/jdk/tree/8248862 - PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators
On Tue, 17 Nov 2020 20:15:34 GMT, Erik Joelsson wrote: >> 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 . > > It looks like you have deleted the doc directory. That can't be right? @erikj79 That's possible I had an intermediate doc folder before the master doc was added. I though I was careful to resolve conflict by selecting master but it might be that I picked the wrong one. Will try and correct in next round,. - PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8256152: tests fail because of ambiguous method resolution
On Tue, 17 Nov 2020 20:01:37 GMT, Stuart Marks wrote:
> Added a cast in the right place, thanks to @jonathan-gibbons.
test/jdk/java/util/stream/boottest/java.base/java/util/stream/DoubleNodeTest.java
line 69:
> 67: assertEquals(list.size(), array.length);
> 68: for (int i = 0; i < array.length; i++)
> 69: assertEquals((Object) array[i], list.get(i));
Can you make it consistent with the other NodeTest implementations e.g.:
private static void assertEqualsListIntArray(List list, int[]
array) {
assertEquals(list.size(), array.length);
for (int i = 0; i < array.length; i++)
assertEquals(array[i], (int) list.get(i));
}
?
-
PR: https://git.openjdk.java.net/jdk/pull/1274
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators
On Tue, 17 Nov 2020 19:58:47 GMT, Jim Laskey wrote: > 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 . It looks like you have deleted the doc directory. That can't be right? - PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8180352: Add Stream.toList() method [v2]
On Tue, 10 Nov 2020 09:34:56 GMT, Peter Levart wrote:
>> I can see that having a separate IMM_LIST_NULLS type might be necessary to
>> preserve the allows-null/disallows-null behaviour of indexOf and lastIndexOf
>> methods...
>>
>> NOTE ALSO that ListN.equals(o) and ListN.hashCode() are inherited from
>> AbstractImmutableList:
>>
>> @Override
>> public boolean equals(Object o) {
>> if (o == this) {
>> return true;
>> }
>>
>> if (!(o instanceof List)) {
>> return false;
>> }
>>
>> Iterator oit = ((List) o).iterator();
>> for (int i = 0, s = size(); i < s; i++) {
>> if (!oit.hasNext() || !get(i).equals(oit.next())) {
>> return false;
>> }
>> }
>> return !oit.hasNext();
>> }
>> and
>> public int hashCode() {
>> int hash = 1;
>> for (int i = 0, s = size(); i < s; i++) {
>> hash = 31 * hash + get(i).hashCode();
>> }
>> return hash;
>> }
>>
>> ...which means they will throw NPE when the list contains null. The same
>> goes for SubList.
>
>> @plevart wrote:
>>
>> > But the question is how does having a separate CollSer.IMM_LIST_NULLS type
>> > prevent that from happening?
>>
>> When a serialized list with IMM_LIST_NULLS is deserialized on an older JDK,
>> it'll throw InvalidObjectException since that tag isn't valid on older JDKs.
>> Obviously this is still an error, but it's a fail-fast approach that avoids
>> letting nulls leak into a data structure where they might cause a problem
>> some arbitrary time later.
>>
>
> Yes, but that is JDK16+ vs. JDK15- and not App V1 vs. App V2 thing. If both
> apps run on JDK16+, there will be no exception.
>
> What I'm trying to say is that the same problem of injecting unexpected nulls
> via serialization/deserialization can happen also if App V2 starts using
> ArrayList to construct the data structure and serialize it while App V1
> deserializes it and expects non-null values only. App V1 would already have
> to guard against null values during deserialization in that case, because
> possibility of null values in deserialized data structure is nothing new for
> App V1.
@plevart wrote:
> Yes, but that is JDK16+ vs. JDK15- and not App V1 vs. App V2 thing. If both
> apps run on JDK16+, there will be no exception.
Sure, the IMM_LIST_NULLS tag only helps with serialization compatibility across
JDK releases. There are lots of ways an app can make incompatible changes to
the serialized forms of its objects that we have no way of detecting.
-
PR: https://git.openjdk.java.net/jdk/pull/1026
RFR: 8256152: tests fail because of ambiguous method resolution
Added a cast in the right place, thanks to @jonathan-gibbons. - Commit messages: - 8256152: tests fail because of ambiguous method resolution Changes: https://git.openjdk.java.net/jdk/pull/1274/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1274=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8256152 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1274.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1274/head:pull/1274 PR: https://git.openjdk.java.net/jdk/pull/1274
RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators
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 . - Commit messages: - 8248862: Implement Enhanced Pseudo-Random Number Generators - Merge branch 'master' into 8248862 - Merge branch 'master' into 8248862 - 8248862: Implement Enhanced Pseudo-Random Number Generators - Merge branch 'master' into 8248862 - 8248862: Implement Enhanced Pseudo-Random Number Generators - 8248862; Implement Enhanced Pseudo-Random Number Generators - Merge branch 'master' into 8248862 - 8248862: Implement Enhanced Pseudo-Random Number Generators - Merge branch 'master' into 8248862 - ... and 27 more: https://git.openjdk.java.net/jdk/compare/9efbb463...7469ca58 Changes: https://git.openjdk.java.net/jdk/pull/1273/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1273=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8248862 Stats: 14875 lines in 31 files changed: 11094 ins; 3704 del; 77 mod Patch: https://git.openjdk.java.net/jdk/pull/1273.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1273/head:pull/1273 PR: https://git.openjdk.java.net/jdk/pull/1273
Re: RFR: 8253459: Formatter treats index, width and precision > Integer.MAX_VALUE incorrectly [v9]
> The `java.util.Formatter` format specifies support for field widths, argument > indexes, or precision lengths of a field that relate to the variadic > arguments supplied to the formatter. These numbers are specified by integers, > sometimes negative. For argument index, it's specified in the documentation > that the highest allowed argument is limited by the largest possible index of > an array (ie the largest possible variadic index), but for the other two it's > not defined. Moreover, what happens when a number field in a string is too > large or too small to be represented by a 32-bit integer type is not defined. > > This fix adds documentation to specify what error behavior occurs during > these cases. Additionally it adds an additional exception type to throw when > an invalid argument index is observed. > > A CSR will be required for this PR. Ian Graves has updated the pull request incrementally with one additional commit since the last revision: Adding test coverage. Tweaking wording in docs. - Changes: - all: https://git.openjdk.java.net/jdk/pull/516/files - new: https://git.openjdk.java.net/jdk/pull/516/files/5a0effe1..aaa35af2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=516=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=516=07-08 Stats: 106 lines in 4 files changed: 103 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/516.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/516/head:pull/516 PR: https://git.openjdk.java.net/jdk/pull/516
Re: 'Find' method for Iterable
On 9/21/2020 4:08 AM, Michael Kuhlmann wrote: But after thinking about it, I'm now convinced that it would be a bad idea. Because it extends the scope of this small, tiny Iterable interface to something bigger which it shouldn't be. This response captures the essence of the problem. You may think you are asking for "just one more method on Iterable", but really, what you are asking for is to turn Iterable into something it is not. Iterable exists not to be "smallest collection", but as the interface to the foreach-loop. Yes, we could have chosen a different design center for Iterable (Eclipse Collections did, see RichIterable), but we didn't. Adding this method merely sends the signal that we want to extend Iterable to be more than it is, which opens the floodgate to requests (and eventually demands) for more methods. I can ask about Iterable#forEach - is it there only because it was there to begin with? Would it have been a bad idea to add one if we had streams already? While you can make an argument that forEach is excessive, the fact that you make this argument illustrates the essential problem with this proposal. Your argument wrt forEach is "If that makes sense, then so does find." If you pull on that string, then this method forms the basis of tomorrow's assumption: "You have forEach and find, it is unreasonable to not add ". So, Iterable should stay simple. (The other arguments against it on this thread, are also valid, but this is the most important one.)
Re: RFR: 8256189: Exact VarHandle tests should test withInvokeBehavior() works as expected
On Tue, 17 Nov 2020 18:32:30 GMT, Jorn Vernee wrote: > This PR sharpens the testing done by > test/jdk/java/lang/invoke/VarHandles/VarHandleTestExact.java after > @mcimadamore reported that the test was not catching an issue with memory > access var handles; namely that the implementation of withInvokeBehavior was > incorrect. > > After some debugging it turned out that the test never actually tested: > (1) going back to invoke behavior from a var handle with invoke exact behavior > (2) the memory access handle implementation of withInvoke(Exact)Behavior, due > to memory handles always being adapted. > > The patch adds testing for (1), and adds a flag to jdk.internal.foreign.Utils > to turn off the adaptation, so that we can test the 'naked' memory access var > handles as well for (2). > > I've also tried to reduce some of the code duplication by creating the higher > order doTest function, that does most of the testing (besides setting up var > handles and test values). Looks good -thanks for catching the issue in the memory access var handle template - Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1267
Re: 'Find' method for Iterable
I think that the discussion is still at the level of if it's an acceptable
addition and not yet at where to implement it. I gave my reasoning in
response to Stuart's points as to what I think is acceptable and what
isn't. Whether the java.lang package has special status is a bit ahead of
where we are now.
On Wed, Nov 11, 2020 at 7:22 PM Justin Dekeyser
wrote:
> Hello all,
>
> Were those downsides seriously considered and strongly argued against? :
>
> (1) Iterable belongs to java.lang so, at least from how it is usually
> approached, it's a feature at the very level of the language, not a
> util feature like Collection. Adding a find on it, is it really
> relevant, or should it be concerned with the simple idea of unlocking
> the enhanced-for loop syntax?
>
> (2) For the find() method to stop on any Iterable, the Iterable should
> have finite size. This mean the size() method can be implemented too,
> by this move:
> ```java
> default int size() {
>int[] dummy = { 0 };
>Predicate falsy = $ -> { dummy[0] += 1; return false; }
>find(falsy);
>return dummy[0];
> }
> ```
> At the very moment an Iterable also has a size(), a Collection
> implementation can be provided (since the altering methods add/remove
> are optional in this spec). So , long story short:
>
> Adding a find() method on Iterable with a contract that it stops
> (stops, and not just linear search time), makes it a Collection. It
> really looks like a nonsense. Or maybe you're going to relax the "this
> method is guaranteed to stop" from the spec? Is it then still
> relevant?
>
> Note: This (2) problem is not a problem for now, because all the utils
> that turns Iterable to a collection like stuff, or a Stream, are
> actually features *on those latter* classes. Hence it's not a concern
> of Iterable itself.
> The problem with (1) is really a communication problem: nothing is
> made in the Java documentation to clearly indicate whether or not
> Iterable should be used businesswise, or is it only for
> syntax-enhancement. (The same problem goes for AutoCloseable: you'll
> really find two schools out of there.)
>
> (3) To my opinion, most people are using Iterable either from a
> collection (where the .stream().filter(p).findAny() makes the job.
> It's not perfect but it's quite clear and flexible) or with a
> ResultSet, where the SQL procedure could/should be used anyway. The
> other cases look so exotic (but I might be wrong) that I really wonder
> how a find method on a Iterable (and not Collection) stuff can be felt
> so useful and language fundamental, that it should be in the java.lang
> package.
>
> Best regards,
>
> Justin Dekeyser
>
> On Tue, Nov 10, 2020 at 7:02 PM Nir Lisker wrote:
> >
> > Did this discussion get lost?
> >
> > On Sun, Sep 20, 2020 at 1:27 AM Nir Lisker wrote:
> >
> > > While it might not be difficult to add a find() method to Iterable, why
> > >> limit it to
> > >> the find operation, and what about all the other operations available
> on
> > >> Stream?
> > >
> > >
> > > Good question. I would say it's a matter of how much it is used and
> what
> > > it takes to implement it. The find operation is a lot more common than
> > > reduce from what I observe, for example, so I wouldn't suggest reduce
> to be
> > > added..A map(Function) operation would require creating a new
> > > collection/iterable internally, and that can be messy (you could
> > > preemptively create and pass your own, but then I doubt the worthiness
> of
> > > it). forEach already exists. I just don't see anything enticing.
> > >
> > > Maybe what's necessary is a way to convert an Iterable to a Stream.
> > >
> > >
> > > Most Iterables are Collections and arrays, and these are easy to
> convert,
> > > so I'm not sure if that really helps. Besides,the idea is to avoid
> Stream,
> > > as I've mentioned, due to the cumbersomeness and the overhead of
> creating a
> > > stream. If I need to do
> > >
> > > iterable.stream().filter(person -> person.id ==
> 123456).findAny/First()
> > >
> > > then I didn't really solve my problem.
> > >
> > > On the other hand, your examples use a list. The List interface already
> > >> has methods
> > >> indexOf/lastIndexOf which search the list for a particular object
> that's
> > >> compared
> > >> using equals(). It seems reasonable to consider similar methods that
> take
> > >> a
> > >> predicate instead of an object.
> > >
> > >
> > > I could have used a Set just as well. As for indexOf(Predicate), I
> > > would say that it is useful (but personally, I hardly ever need the
> index
> > > of an object, I need the object itself). Interestingly,
> > > removeIf(Predicate) exists, but remove(Predicate) doesn't. I
> would
> > > think twice before suggesting to add it though.
> > >
> > > Ultimately, you have access to a lot of analytics and codebase scans.
> If
> > > you know which patterns are used a lot more than others it would be a
> good
> > > guide. If there are a lot of iterations in order to find an
RFR: 8256189: Exact VarHandle tests should test withInvokeBehavior() works as expected
This PR sharpens the testing done by test/jdk/java/lang/invoke/VarHandles/VarHandleTestExact.java after @mcimadamore reported that the test was not catching an issue with memory access var handles; namely that the implementation of withInvokeBehavior was incorrect. After some debugging it turned out that the test never actually tested: (1) going back to invoke behavior from a var handle with invoke exact behavior (2) the memory access handle implementation of withInvoke(Exact)Behavior, due to memory handles always being adapted. The patch adds testing for (1), and adds a flag to jdk.internal.foreign.Utils to turn off the adaptation, so that we can test the 'naked' memory access var handles as well for (2). I've also tried to reduce some of the code duplication by creating the higher order doTest function, that does most of the testing (besides setting up var handles and test values). - Commit messages: - Fix memory handles test to use un-adapted var handles, so we can test the invoke exact behaviour on 'naked' memory handles - - Reduce VarHandleTestExact code duplication Changes: https://git.openjdk.java.net/jdk/pull/1267/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1267=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8256189 Stats: 173 lines in 3 files changed: 48 ins; 80 del; 45 mod Patch: https://git.openjdk.java.net/jdk/pull/1267.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1267/head:pull/1267 PR: https://git.openjdk.java.net/jdk/pull/1267
Re: RFR: 8256477: Specialize heap memory segment implementations
On Tue, 17 Nov 2020 14:55:07 GMT, Maurizio Cimadamore wrote: > The current memory segment implementation defines a hierarchy with 3 concrete > classes: one for heap segments, one for native segments and one for mapped > segments. > > Since there can be many kinds of heap segments (e.g. created from a byte[] or > from a float[]) the current implementation is prone to type profile pollution > problems: if enough heap segments are created (of different kinds), the JIT > compiler will give up on speculating on the heap segment kind, which will > then result in poor performances. > > This issue can be reproduced in one of the existing benchmark, by adding some > initialization code which is enough to pollute the types profiles. When that > happens, performance numbers look like the following: > > Benchmark (polluteProfile) Mode Cnt Score > Error Units > LoopOverNonConstantHeap.segment_loop false avgt 10 0.285 ± > 0.003 ms/op > LoopOverNonConstantHeap.segment_loop true avgt 10 5.540 ± > 0.143 ms/op > > (Thanks to Vlad for coming up for the exact incantation which leads to > profile pollution :-) ) > > The solution is to create a sharp subclass for each heap segment case. With > this, C2 has always a sharp Unsafe *base* to work with, and performances are > stable regardless of profile pollution attempts. > > This patch also tweaks the benchmark for heap segments so that it checks it > with and without profile pollution. Marked as reviewed by jvernee (Committer). - PR: https://git.openjdk.java.net/jdk/pull/1259
Integrated: 8256370: Add asserts to Reference.getInactive()
On Mon, 16 Nov 2020 17:29:20 GMT, Roman Kennke wrote: > A follow-up to JDK-8256106, this is adding two asserts to check that the API > is used as it should be, i.e. only on inactive FinalReferences. Also, in > Finalizer, where getInactive() is used, there is a null-check. The GC must > never clean the referent, and Java code doesn't clean it either, it would be > a bug if we ever see null there. I think it's better to fail there (with > assert or NPE) when that happens instead of silently accepting it. > > Testing: > - [x] tier1 > - [x] tier2 This pull request has now been integrated. Changeset: f2a9d02d Author:Roman Kennke URL: https://git.openjdk.java.net/jdk/commit/f2a9d02d Stats: 4 lines in 2 files changed: 3 ins; 0 del; 1 mod 8256370: Add asserts to Reference.getInactive() Reviewed-by: mchung - PR: https://git.openjdk.java.net/jdk/pull/1231
RFR: 8256477: Specialize heap memory segment implementations
The current memory segment implementation defines a hierarchy with 3 concrete classes: one for heap segments, one for native segments and one for mapped segments. Since there can be many kinds of heap segments (e.g. created from a byte[] or from a float[]) the current implementation is prone to type profile pollution problems: if enough heap segments are created (of different kinds), the JIT compiler will give up on speculating on the heap segment kind, which will then result in poor performances. This issue can be reproduced in one of the existing benchmark, by adding some initialization code which is enough to pollute the types profiles. When that happens, performance numbers look like the following: Benchmark (polluteProfile) Mode Cnt Score Error Units LoopOverNonConstantHeap.segment_loop false avgt 10 0.285 ± 0.003 ms/op LoopOverNonConstantHeap.segment_loop true avgt 10 5.540 ± 0.143 ms/op (Thanks to Vlad for coming up for the exact incantation which leads to profile pollution :-) ) The solution is to create a sharp subclass for each heap segment case. With this, C2 has always a sharp Unsafe *base* to work with, and performances are stable regardless of profile pollution attempts. This patch also tweaks the benchmark for heap segments so that it checks it with and without profile pollution. - Commit messages: - Tweak code so that specialized classes define their own factories - make base() method abstract in HeapMemorySegmentImpl - Create specialized subclasses for all the heap segment cases Changes: https://git.openjdk.java.net/jdk/pull/1259/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1259=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8256477 Stats: 187 lines in 4 files changed: 140 ins; 4 del; 43 mod Patch: https://git.openjdk.java.net/jdk/pull/1259.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1259/head:pull/1259 PR: https://git.openjdk.java.net/jdk/pull/1259
Re: RFR: JDK-8255055: Create two phase test for case with different names, and fix linux DTI
On Tue, 17 Nov 2020 02:01:32 GMT, Alexey Semenyuk wrote:
>> In scenario 1, the installer would be Bar-1.0.{exe, msi, pkg ...}
>> The change in test above tests this.
>> Scenario 2 would be an error currently, since all jpackage commands require
>> a "--name" argument, however, it would be reasonable to modify restriction
>> to require either a name or an app-image, extracting the name from the
>> app-image in this case.
>
> I think we need to define the behavior we want from two phase jpackage runs
> first and create tests after that. I agree it would be good to extract name
> from app image if it is not specified explicitly when building an installer
I see that further changes are needed in jpackage code to properly handle all
cases of installer name vs application name.
I will open a new bug to fix all such cases, and include test cases
I will then close this bug as dupe of that, and I am closing this pull request
-
PR: https://git.openjdk.java.net/jdk/pull/1229
Withdrawn: JDK-8255055: Create two phase test for case with different names, and fix linux DTI
On Mon, 16 Nov 2020 15:15:02 GMT, Andy Herrick wrote: > … fix linux DTI This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/1229
Re: RFR: 8256435: [TESTBUG] java/foreign/TestHandshake.java fails with direct buffer memory OOM
On Tue, 17 Nov 2020 09:07:03 GMT, Nick Gasson wrote:
> I ran this test on a machine with 224 logical CPUs and it fails with:
>
> ITERATION 3
> test TestHandshake.testHandshake("SegmentMismatchAccessor",
> TestHandshake$$Lambda$57/0x0001000e7968@37c4b344): failure
> java.lang.OutOfMemoryError: Cannot reserve 100 bytes of direct buffer
> memory (allocated: 536008192, limit: 536870912)
>
> SegmentMismatchAccessor allocates a 1MB native segment for each CPU on
> each iteration. This can quickly reach the allocation limit if there is
> no intervening GC. Explicitly close the segment after each iteration to
> release the memory.
Whoops - good catch!
-
Marked as reviewed by mcimadamore (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/1254
Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v28]
> This patch contains the changes associated with the first incubation round of > the foreign linker access API incubation > (see JEP 389 [1]). This work is meant to sit on top of the foreign memory > access support (see JEP 393 [2] and associated pull request [3]). > > The main goal of this API is to provide a way to call native functions from > Java code without the need of intermediate JNI glue code. In order to do > this, native calls are modeled through the MethodHandle API. I suggest > reading the writeup [4] I put together few weeks ago, which illustrates what > the foreign linker support is, and how it should be used by clients. > > Disclaimer: the pull request mechanism isn't great at managing *dependent* > reviews. For this reasons, I'm attaching a webrev which contains only the > differences between this PR and the memory access PR. I will be periodically > uploading new webrevs, as new iterations come out, to try and make the life > of reviewers as simple as possible. > > A big thank to Jorn Vernee and Vladimir Ivanov - they are the main architects > of all the hotspot changes you see here, and without their help, the foreign > linker support wouldn't be what it is today. As usual, a big thank to Paul > Sandoz, who provided many insights (often by trying the bits first hand). > > Thanks > Maurizio > > Webrev: > http://cr.openjdk.java.net/~mcimadamore/8254231_v1/webrev > > Javadoc: > > http://cr.openjdk.java.net/~mcimadamore/8254231_v1/javadoc/jdk/incubator/foreign/package-summary.html > > Specdiff (relative to [3]): > > http://cr.openjdk.java.net/~mcimadamore/8254231_v1/specdiff_delta/overview-summary.html > > CSR: > > https://bugs.openjdk.java.net/browse/JDK-8254232 > > > > ### API Changes > > The API changes are actually rather slim: > > * `LibraryLookup` > * This class allows clients to lookup symbols in native libraries; the > interface is fairly simple; you can load a library by name, or absolute path, > and then lookup symbols on that library. > * `FunctionDescriptor` > * This is an abstraction that is very similar, in spirit, to `MethodType`; > it is, at its core, an aggregate of memory layouts for the function > arguments/return type. A function descriptor is used to describe the > signature of a native function. > * `CLinker` > * This is the real star of the show. A `CLinker` has two main methods: > `downcallHandle` and `upcallStub`; the first takes a native symbol (as > obtained from `LibraryLookup`), a `MethodType` and a `FunctionDescriptor` and > returns a `MethodHandle` instance which can be used to call the target native > symbol. The second takes an existing method handle, and a > `FunctionDescriptor` and returns a new `MemorySegment` corresponding to a > code stub allocated by the VM which acts as a trampoline from native code to > the user-provided method handle. This is very useful for implementing upcalls. >* This class also contains the various layout constants that should be > used by clients when describing native signatures (e.g. `C_LONG` and > friends); these layouts contain additional ABI classfication information (in > the form of layout attributes) which is used by the runtime to *infer* how > Java arguments should be shuffled for the native call to take place. > * Finally, this class provides some helper functions e.g. so that clients > can convert Java strings into C strings and back. > * `NativeScope` > * This is an helper class which allows clients to group together logically > related allocations; that is, rather than allocating separate memory segments > using separate *try-with-resource* constructs, a `NativeScope` allows clients > to use a _single_ block, and allocate all the required segments there. This > is not only an usability boost, but also a performance boost, since not all > allocation requests will be turned into `malloc` calls. > * `MemorySegment` > * Only one method added here - namely `handoff(NativeScope)` which allows a > segment to be transferred onto an existing native scope. > > ### Safety > > The foreign linker API is intrinsically unsafe; many things can go wrong when > requesting a native method handle. For instance, the description of the > native signature might be wrong (e.g. have too many arguments) - and the > runtime has, in the general case, no way to detect such mismatches. For these > reasons, obtaining a `CLinker` instance is a *restricted* operation, which > can be enabled by specifying the usual JDK property > `-Dforeign.restricted=permit` (as it's the case for other restricted method > in the foreign memory API). > > ### Implementation changes > > The Java changes associated with `LibraryLookup` are relative > straightforward; the only interesting thing to note here is that library > loading does _not_ depend on class loaders, so `LibraryLookup` is not subject > to the same restrictions which apply to JNI library loading (e.g. same > library
Integrated: 8202471: (ann) Cannot read type annotations on generic receiver type's type variables
On Sat, 24 Oct 2020 21:44:22 GMT, Rafael Winterhalter wrote: > A method's or constructor's owner type might carry annotations on its > potential type parameters but is never represented as parameterized type what > makes these parameters inaccessible at runtime, despite the presence of > parameter type annotations. This pull request has now been integrated. Changeset: 53a31889 Author:Rafael Winterhalter Committer: Joel Borggrén-Franck URL: https://git.openjdk.java.net/jdk/commit/53a31889 Stats: 242 lines in 7 files changed: 235 ins; 0 del; 7 mod 8202471: (ann) Cannot read type annotations on generic receiver type's type variables Reviewed-by: jfranck - PR: https://git.openjdk.java.net/jdk/pull/851
Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v26]
On Mon, 16 Nov 2020 18:44:55 GMT, Rajan Halade wrote:
>> Maurizio Cimadamore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix aarch64 test failure
>
> src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 387:
>
>> 385: }
>> 386:
>> 387: public static NativeLibrary defaultLibrary = new
>> NativeLibraryImpl(Object.class, "", true, true) {
>
> This field can be final.
Thanks - I already made this change in the latest revision.
-
PR: https://git.openjdk.java.net/jdk/pull/634
RFR: 8255918: XMLStreamFilterImpl constructor consumes XMLStreamException
Allow `XMLStreamException` to be thrown to the application when a filtered `XMLStreamReader` encounters an `XMLStreamException` advancing the wrapped `XMLStreamReader`. Note, this PR includes a method signature change (constructor of `com.sun.org.apache.xerces.internal.impl.XMLStreamFilterImpl`). - Commit messages: - 8255918: Throw XMLStreamExceptions from XMLStreamFilterImpl constructor Changes: https://git.openjdk.java.net/jdk/pull/1209/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1209=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255918 Stats: 100 lines in 2 files changed: 88 ins; 7 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/1209.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1209/head:pull/1209 PR: https://git.openjdk.java.net/jdk/pull/1209
Integrated: 8255150: Add utility methods to check long indexes and ranges
On Mon, 2 Nov 2020 10:47:18 GMT, Roland Westrelin wrote: > This change add 3 new methods in Objects: > > public static long checkIndex(long index, long length) > public static long checkFromToIndex(long fromIndex, long toIndex, long length) > public static long checkFromIndexSize(long fromIndex, long size, long length) > > This mirrors the int utility methods that were added by JDK-8135248 > with the same motivations. > > As is the case with the int checkIndex(), the long checkIndex() method > is JIT compiled as an intrinsic. It allows the JIT to compile > checkIndex to an unsigned comparison and properly recognize it as > a range check that then becomes a candidate for the existing range check > optimizations. This has proven to be important for panama's > MemorySegment API and a prototype of this change (with some extra c2 > improvements) showed that panama micro benchmark results improve > significantly. > > This change includes: > > - the API change > - the C2 intrinsic > - tests for the API and the C2 intrinsic > > This is a joint work with Paul who reviewed and reworked the API change > and filled the CSR. This pull request has now been integrated. Changeset: a7422ac2 Author:Roland Westrelin URL: https://git.openjdk.java.net/jdk/commit/a7422ac2 Stats: 897 lines in 30 files changed: 846 ins; 4 del; 47 mod 8255150: Add utility methods to check long indexes and ranges Co-authored-by: Paul Sandoz Reviewed-by: jvernee, dlong, vlivanov - PR: https://git.openjdk.java.net/jdk/pull/1003
Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v7]
On Tue, 17 Nov 2020 08:33:20 GMT, Roland Westrelin wrote: >> This change add 3 new methods in Objects: >> >> public static long checkIndex(long index, long length) >> public static long checkFromToIndex(long fromIndex, long toIndex, long >> length) >> public static long checkFromIndexSize(long fromIndex, long size, long length) >> >> This mirrors the int utility methods that were added by JDK-8135248 >> with the same motivations. >> >> As is the case with the int checkIndex(), the long checkIndex() method >> is JIT compiled as an intrinsic. It allows the JIT to compile >> checkIndex to an unsigned comparison and properly recognize it as >> a range check that then becomes a candidate for the existing range check >> optimizations. This has proven to be important for panama's >> MemorySegment API and a prototype of this change (with some extra c2 >> improvements) showed that panama micro benchmark results improve >> significantly. >> >> This change includes: >> >> - the API change >> - the C2 intrinsic >> - tests for the API and the C2 intrinsic >> >> This is a joint work with Paul who reviewed and reworked the API change >> and filled the CSR. > > Roland Westrelin has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 27 commits: > > - Merge branch 'master' of https://git.openjdk.java.net/jdk into JDK-8255150 > - Merge branch 'master' into JDK-8255150 > - Merge branch 'master' into JDK-8255150 > - Merge branch 'master' of https://git.openjdk.java.net/jdk into JDK-8255150 > - exclude compiler test when run with -Xcomp > - CastLL should define carry_depency > - intrinsic comments > - Jorn's comments > - Update headers and add intrinsic to Graal test ignore list > - move compiler test and add bug to test > - ... and 17 more: > https://git.openjdk.java.net/jdk/compare/4553fa0b...feb32943 Marked as reviewed by jvernee (Committer). - PR: https://git.openjdk.java.net/jdk/pull/1003
Re: RFR: 8202471: (ann) Cannot read type annotations on generic receiver type's type variables [v7]
On Mon, 16 Nov 2020 11:51:32 GMT, Rafael Winterhalter wrote: >> I think it would be good to rebase on top of a recent jdk upstream before >> merging. > > Rebased on HEAD/master. Tier 1-3 looks good, go ahead and / integrate and I'll sponsor - PR: https://git.openjdk.java.net/jdk/pull/851
Re: RFR: 8256435: [TESTBUG] java/foreign/TestHandshake.java fails with direct buffer memory OOM
On Tue, 17 Nov 2020 09:07:03 GMT, Nick Gasson wrote:
> I ran this test on a machine with 224 logical CPUs and it fails with:
>
> ITERATION 3
> test TestHandshake.testHandshake("SegmentMismatchAccessor",
> TestHandshake$$Lambda$57/0x0001000e7968@37c4b344): failure
> java.lang.OutOfMemoryError: Cannot reserve 100 bytes of direct buffer
> memory (allocated: 536008192, limit: 536870912)
>
> SegmentMismatchAccessor allocates a 1MB native segment for each CPU on
> each iteration. This can quickly reach the allocation limit if there is
> no intervening GC. Explicitly close the segment after each iteration to
> release the memory.
@mcimadamore I'm not sure whether I should submit this here or panama-foreign,
but the fix applies to jdk master.
-
PR: https://git.openjdk.java.net/jdk/pull/1254
RFR: 8256435: [TESTBUG] java/foreign/TestHandshake.java fails with direct buffer memory OOM
I ran this test on a machine with 224 logical CPUs and it fails with:
ITERATION 3
test TestHandshake.testHandshake("SegmentMismatchAccessor",
TestHandshake$$Lambda$57/0x0001000e7968@37c4b344): failure
java.lang.OutOfMemoryError: Cannot reserve 100 bytes of direct buffer
memory (allocated: 536008192, limit: 536870912)
SegmentMismatchAccessor allocates a 1MB native segment for each CPU on
each iteration. This can quickly reach the allocation limit if there is
no intervening GC. Explicitly close the segment after each iteration to
release the memory.
-
Commit messages:
- 8256435: [TESTBUG] java/foreign/TestHandshake.java fails with direct buffer
memory OOM
Changes: https://git.openjdk.java.net/jdk/pull/1254/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1254=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8256435
Stats: 9 lines in 1 file changed: 8 ins; 0 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/1254.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/1254/head:pull/1254
PR: https://git.openjdk.java.net/jdk/pull/1254
Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v7]
On Tue, 17 Nov 2020 08:33:20 GMT, Roland Westrelin wrote: >> This change add 3 new methods in Objects: >> >> public static long checkIndex(long index, long length) >> public static long checkFromToIndex(long fromIndex, long toIndex, long >> length) >> public static long checkFromIndexSize(long fromIndex, long size, long length) >> >> This mirrors the int utility methods that were added by JDK-8135248 >> with the same motivations. >> >> As is the case with the int checkIndex(), the long checkIndex() method >> is JIT compiled as an intrinsic. It allows the JIT to compile >> checkIndex to an unsigned comparison and properly recognize it as >> a range check that then becomes a candidate for the existing range check >> optimizations. This has proven to be important for panama's >> MemorySegment API and a prototype of this change (with some extra c2 >> improvements) showed that panama micro benchmark results improve >> significantly. >> >> This change includes: >> >> - the API change >> - the C2 intrinsic >> - tests for the API and the C2 intrinsic >> >> This is a joint work with Paul who reviewed and reworked the API change >> and filled the CSR. > > Roland Westrelin has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 27 commits: > > - Merge branch 'master' of https://git.openjdk.java.net/jdk into JDK-8255150 > - Merge branch 'master' into JDK-8255150 > - Merge branch 'master' into JDK-8255150 > - Merge branch 'master' of https://git.openjdk.java.net/jdk into JDK-8255150 > - exclude compiler test when run with -Xcomp > - CastLL should define carry_depency > - intrinsic comments > - Jorn's comments > - Update headers and add intrinsic to Graal test ignore list > - move compiler test and add bug to test > - ... and 17 more: > https://git.openjdk.java.net/jdk/compare/4553fa0b...feb32943 Marked as reviewed by dlong (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1003
Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v7]
> This change add 3 new methods in Objects: > > public static long checkIndex(long index, long length) > public static long checkFromToIndex(long fromIndex, long toIndex, long length) > public static long checkFromIndexSize(long fromIndex, long size, long length) > > This mirrors the int utility methods that were added by JDK-8135248 > with the same motivations. > > As is the case with the int checkIndex(), the long checkIndex() method > is JIT compiled as an intrinsic. It allows the JIT to compile > checkIndex to an unsigned comparison and properly recognize it as > a range check that then becomes a candidate for the existing range check > optimizations. This has proven to be important for panama's > MemorySegment API and a prototype of this change (with some extra c2 > improvements) showed that panama micro benchmark results improve > significantly. > > This change includes: > > - the API change > - the C2 intrinsic > - tests for the API and the C2 intrinsic > > This is a joint work with Paul who reviewed and reworked the API change > and filled the CSR. Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 27 commits: - Merge branch 'master' of https://git.openjdk.java.net/jdk into JDK-8255150 - Merge branch 'master' into JDK-8255150 - Merge branch 'master' into JDK-8255150 - Merge branch 'master' of https://git.openjdk.java.net/jdk into JDK-8255150 - exclude compiler test when run with -Xcomp - CastLL should define carry_depency - intrinsic comments - Jorn's comments - Update headers and add intrinsic to Graal test ignore list - move compiler test and add bug to test - ... and 17 more: https://git.openjdk.java.net/jdk/compare/4553fa0b...feb32943 - Changes: https://git.openjdk.java.net/jdk/pull/1003/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1003=06 Stats: 897 lines in 30 files changed: 846 ins; 4 del; 47 mod Patch: https://git.openjdk.java.net/jdk/pull/1003.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1003/head:pull/1003 PR: https://git.openjdk.java.net/jdk/pull/1003
