Re: RFR: 8251989: Hex formatting and parsing utility [v8]
On Fri, 23 Oct 2020 16:34:54 GMT, Roger Riggs wrote: >> Do you want to refactor some of the classes where `HexFormat` would be a >> well-suited replacement for the current code, and what do you think about my >> other previous review comments (mostly formatting related)? >> The bridgekeeper bot was so kind and censored my [previous >> comment](https://github.com/openjdk/jdk/pull/482#issuecomment-710709003), >> but apparently it can't handle review comments yet ... >> >> And no, I am not going to sign the [OpenJDK Terms of >> Use](https://openjdk.java.net/legal/tou/terms) for a simple pull request >> comment, especially when the Terms of Use document is quite long and >> contains the following: >>> You hereby grant to Oracle and all Users a royalty-free, perpetual, >>> irrevocable, worldwide, non-exclusive and fully sub-licensable right and >>> license under Your intellectual property rights to reproduce, modify, >>> adapt, publish, translate, create derivative works from, distribute, >>> perform, display and use Your Submissions (in whole or part) and to >>> incorporate or implement them in other works in any form, media, or >>> technology now known or later developed. This includes, without limitation, >>> the right to incorporate or implement the Submission into any product or >>> service, and to display, market, sublicense and distribute the Submissions >>> as incorporated or embedded in any product or service distributed or >>> offered by Oracle without compensation to you. >> >> (Might be good to rework that to actually encourage contributions. I am kind >> of surprised that any company is willing to contribute to the OpenJDK under >> these terms.) >> >> Sorry for driving this pull request off-topic, I will stop all interaction >> here and will not bother you any further here if that is desired. Below is >> the original comment: >> >> - >> >> It might also be good to clarify in the documentation that `parseHex(...)` >> is case insensitive. This is currently not obvious and one might expect that >> it respects `isUpperCase()`. >> >> Additionally there are quite a lot places within the JDK code where this new >> class could be used. I have picked some where it would definitely be an >> improvement (but omitted test classes): >> >> Classes >> >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/java/net/HostPortrange.java#L94;>java.base >> java.net.HostPortrange >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/jdk/internal/module/ModuleHashes.java#L215;>java.base >> jdk.internal.module.ModuleHashes >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java#L544;>java.base >> sun.net.www.protocol.http.DigestAuthentication >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/provider/AbstractDrbg.java#L738;>java.base >> sun.security.provider.AbstractDrbg >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/provider/certpath/ResponderId.java#L269;>java.base >> sun.security.provider.certpath.ResponderId >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/ssl/Utilities.java#L39;>java.base >> sun.security.ssl.Utilities >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/Debug.java#L323;>java.base >> sun.security.util.Debug >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/HexDumpEncoder.java;>java.base >> sun.security.util.HexDumpEncoder >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/ManifestEntryVerifier.java#L244;>java.base >> sun.security.util.ManifestEntryVerifier >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java#L753;>java.base >> sun.security.util.SignatureFileVerifier >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L742;>java.base >> sun.security.x509.AVA >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L811;>java.base >> sun.security.x509.AVA >> > href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L889;>java.base >> sun.security.x509.AVA >> >
Re: RFR: 8251989: Hex formatting and parsing utility [v9]
> java.util.HexFormat utility: > > - Format and parse hexadecimal strings, with parameters for delimiter, > prefix, suffix and upper/lowercase > - Static factories and builder methods to create HexFormat copies with > modified parameters. > - Consistent naming of methods for conversion of byte arrays to formatted > strings and back: formatHex and parseHex > - Consistent naming of methods for conversion of primitive types: > toHexDigits... and fromHexDigits... > - Prefix and suffixes now apply to each formatted value, not the string as a > whole > - Using java.util.Appendable as a target for buffered conversions so output > to Writers and PrintStreams >like System.out are supported in addition to StringBuilder. (IOExceptions > are converted to unchecked exceptions) > - Immutable and thread safe, a "value-based" class > > See the [HexFormat > javadoc](http://cr.openjdk.java.net/~rriggs/8251989-hex-formatter/java.base/java/util/HexFormat.html) > for details. > > Review comments and suggestions welcome. Roger Riggs has updated the pull request incrementally with two additional commits since the last revision: - The HexFormat API indexing model for array and string ranges is changed to describe the range using 'fromIndex (inclusive)' and 'toIndex (exclusive)'. Initially, it was specified as 'index' and 'length'. However, both byte arrays and strings used in the HexFormat API typically use fromIndex and toIndex to describe ranges. Using the same indexing model can prevent mistakes. The change affects the methods and corresponding tests: formatHex(byte[] bytes, int fromIndex, int toIndex) formatHex(A out, byte[] bytes, int fromIndex, int toIndex) parseHex(char[] chars, int fromIndex, int toIndex) parseHex(CharSequence string, int fromIndex, int toIndex) fromHexDigits(CharSequence string, int fromIndex, int toIndex) fromHexDigitsToLong(CharSequence string, int fromIndex, int toIndex) - - Added @see and @link references to Integer.toHexString and Long.toHexString - Clarified parsing is case insensistive in various parse and fromXXX methods - Source level cleanup based on review comments - Expanded some javadoc tag text to make it more descriptive - Consistent use of 'hexadecimal' vs 'hex' - Changes: - all: https://git.openjdk.java.net/jdk/pull/482/files - new: https://git.openjdk.java.net/jdk/pull/482/files/2b493d37..2aeab7d7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=482=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=482=07-08 Stats: 205 lines in 4 files changed: 25 ins; 8 del; 172 mod Patch: https://git.openjdk.java.net/jdk/pull/482.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/482/head:pull/482 PR: https://git.openjdk.java.net/jdk/pull/482
Re: RFR: 8159746: (proxy) Support for default methods
On Fri, 23 Oct 2020 10:32:27 GMT, Peter Levart wrote: >>> NewInvocationHandler/DefaultMethodInvoker feels a bit awkward. If I have it >>> right, to use it as a lambda expression to Proxy.newProxyInterface would >>> require casting to NewInvocationHandler. I also need to be careful when >>> using the invoker and not call it with a non-default method. >> >> Yes it requires casting to `NewInvocationHandler` or assign the lambda in a >> local variable. > > Hi Mandy, > > I think what matters here is a public surface area - what user sees. The API > friendliness is the 1st aspect and very close to that is performance on the > 2nd place. How complicated things are in the back is not unimportant, but > very far from the leading two. I think this is how alternatives should be > compared. > > What I have done in my last attempt (https://github.com/mlchung/jdk/pull/4) > is, admittedly, quite large from API surface. New `InvocationHandler2` > interface taking additional `InvocationHandler2.SuperInvoker` callback + new > overloaded static factory method `Proxy.newProxyInstance`. Additionally, > internal checks needed to verify access at `newProxyInstance`-time are quite > complicated since `InvocationHandler2` is a super-type of `InvocationHandler` > and therefore the code has to check that the default implementation of > `InvocationHandler.invoke(SuperInvoker, ...)` method is not overridden or > else additional checks must be performed. This leads to complicated > specification as to when access exceptions are thrown. From performance > perspective, this variant showed least per-invocation overhead so far: > > Benchmark Mode Cnt Score Error Units > > ProxyBench.implClassavgt5 3.777 ± 0.003 ns/op > ProxyBench.implProxyavgt5 20.282 ± 0.585 ns/op > ProxyBench.ppImplClass avgt5 3.752 ± 0.002 ns/op > ProxyBench.ppImplProxy avgt5 19.790 ± 0.335 ns/op > > Your `DelegatingInvocationHandler` abstract class is nice and simple, but > unfortunately not lambda-friendly. Will this hinder its use? I don't know. > Alan says that this feature will mostly be for "experts", but even they like > to use lambdas. The performance is quite OK as runtime per-invocation checks > are not very expensive, just ~ 50% slower than my attempt above. Note that > the allocation of 96 bytes per invocation is due to boxing 3 primitive > argument ints into Integers which is unavoidable: > > BenchmarkMode Cnt Score > Error Units > > ProxyBench.implClass avgt5 3.765 > ±0.028 ns/op > ProxyBench.implProxy avgt529.812 > ±0.180 ns/op > ProxyBench.ppImplClass avgt5 3.705 > ±0.110 ns/op > ProxyBench.ppImplProxy avgt531.209 > ±0.900 ns/op > > ProxyBench.implClass:·gc.alloc.rate.norm avgt5≈ 10⁻⁶ > B/op > ProxyBench.implProxy:·gc.alloc.rate.norm avgt596.004 > ±0.001B/op > ProxyBench.ppImplClass:·gc.alloc.rate.norm avgt5≈ 10⁻⁶ > B/op > ProxyBench.ppImplProxy:·gc.alloc.rate.norm avgt596.004 > ±0.001B/op > > (NOTE: I noticed a possibility of a concurrent race condition in > `Proxy.newProxyInstance` that could be used to fool the proxy interfaces > access check - the array of interfaces is not defensively cloned) > > Your `NewInvocationHandler` as a subinterface of `InvocationHandler` that > overrides the InvocationHandler.invoke(Object proxy, ... and wires it to new > abstract method taking additional DefaultMethodInvoker parameter seems > similar to my last attempt, just subtype roles of old and new interfaces are > reversed. This means it is easier to check whether the invocation handler has > access to the super default methods or not (handler instanceof > NewInvocationHandler) and adjust access checks in Proxy.newProxyInstance > accordingly. But, as a consequence, the DefaultMethodInvoker instance has to > be created for each invocation. You could also add an overloaded > Proxy.newProxyInstance method to accept a lambda with 4 parameters > (NewInvocationHandler), so this variant could be made user-friendly too, not > requiring a cast. The performance is a little slower then abstract handler > class. Also, the allocation of new instance of DefaultMethodInvoker per > invocation seems unable to be optimized away by JI T escape analysis (maybe the DefaultMethodInvoker.invoke is to big to be inlined), so we have 16 additional bytes allocated per invocation: > > BenchmarkMode Cnt Score >Error Units > > ProxyBench.implClass avgt5 3.760 > ± 0.049 ns/op >
Re: RFR: 8255231: Avoid upcalls when initializing the statSampler [v4]
On Mon, 26 Oct 2020 12:49:43 GMT, Claes Redestad wrote: >> Current implementation of the statSampler does upcalls to System.getProperty >> to collect values for a number of properties that are all provided by the VM >> itself. And since the sampling starts before any user code run then no >> property can have changed. >> >> I suggest refactoring the code so that no upcalls are made normally - while >> asserting this invariant holds using assert-only upcalls. >> >> This is a small startup optimization - reducing the startup sequence by >> approx. 300k instructions and 70k branches in my linux-x64 setup. > > Claes Redestad 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 21 additional > commits since the last revision: > > - Address review comments from David Holmes > - Merge branch 'master' into com_ns > - Refactor to remove stable_java_property_counters and clarify comments > - Merge branch 'master' into com_ns > - Revert unrelated changes to perfData > - Merge branch 'master' into com_ns > - Improve comments > - typo > - Missing definition > - Extract the shorthand java.version from VersionProps and use it in > StatSampler > - ... and 11 more: > https://git.openjdk.java.net/jdk/compare/145a3876...8572159f Thanks for making the suggested changes. I think we need a further RFE to add some error checking for the sizes of the various property strings in relation to the fixed size arrays that have been allocated to them. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/802
Integrated: 8255206: [macos] LicenseTest fails on macOS 11
On Sat, 24 Oct 2020 00:49:12 GMT, Alexander Matveev wrote: > I did not reproduce issue with license is not shown as per SQE report. It > work as expected. However, jpackage fails due to unflatten/flatten being > removed from hdiutil on macOS 11. I am not sure why it was needed, probably > some legacy code. Without unflatten/flatten everything works as expected on > macOS 11 and 10.5.7. This pull request has now been integrated. Changeset: 8ca59c9e Author:Alexander Matveev URL: https://git.openjdk.java.net/jdk/commit/8ca59c9e Stats: 18 lines in 1 file changed: 0 ins; 18 del; 0 mod 8255206: [macos] LicenseTest fails on macOS 11 Reviewed-by: asemenyuk, herrick - PR: https://git.openjdk.java.net/jdk/pull/847
Re: RFR: 8238263: Create at-requires mechanism for containers [v2]
On Mon, 26 Oct 2020 20:24:35 GMT, Igor Ignatyev wrote: >> Harold Seigel has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8238263: Create at-requires mechanism for containers > > LGTM, modulo my earlier comment/doubt about the name, but I don’t think it’s > important enough Thanks Bob and Igor! - PR: https://git.openjdk.java.net/jdk/pull/844
Integrated: 8238263: Create at-requires mechanism for containers
On Fri, 23 Oct 2020 18:44:54 GMT, Harold Seigel wrote: > Please review this change to add an @requires mechanism called > "jdk.containerized" to help mark tests that are incompatible with containers. > Users would add "@requires jdk.containerized != true" to the incompatible > tests and then use "make test ... OPTIONS=-Djdk.containerized=true" or "bash > jib.sh mach5 -- remote-build-and-test ... --test-make-args > JTREG=OPTIONS=-Djdk.containerized=true" to exclude those tests when testing > with containers. This pull request has now been integrated. Changeset: ca8bba64 Author:Harold Seigel URL: https://git.openjdk.java.net/jdk/commit/ca8bba64 Stats: 10 lines in 3 files changed: 8 ins; 0 del; 2 mod 8238263: Create at-requires mechanism for containers Reviewed-by: bobv, iignatyev - PR: https://git.openjdk.java.net/jdk/pull/844
Re: RFR: 8238263: Create at-requires mechanism for containers [v2]
On Mon, 26 Oct 2020 18:13:29 GMT, Harold Seigel wrote: >> Please review this change to add an @requires mechanism called >> "jdk.containerized" to help mark tests that are incompatible with >> containers. Users would add "@requires jdk.containerized != true" to the >> incompatible tests and then use "make test ... >> OPTIONS=-Djdk.containerized=true" or "bash jib.sh mach5 -- >> remote-build-and-test ... --test-make-args >> JTREG=OPTIONS=-Djdk.containerized=true" to exclude those tests when testing >> with containers. > > Harold Seigel has updated the pull request incrementally with one additional > commit since the last revision: > > 8238263: Create at-requires mechanism for containers LGTM, modulo my earlier comment/doubt about the name, but I don’t think it’s important enough - Marked as reviewed by iignatyev (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/844
Re: RFR: 8238263: Create at-requires mechanism for containers [v2]
On Mon, 26 Oct 2020 18:13:29 GMT, Harold Seigel wrote: >> Please review this change to add an @requires mechanism called >> "jdk.containerized" to help mark tests that are incompatible with >> containers. Users would add "@requires jdk.containerized != true" to the >> incompatible tests and then use "make test ... >> OPTIONS=-Djdk.containerized=true" or "bash jib.sh mach5 -- >> remote-build-and-test ... --test-make-args >> JTREG=OPTIONS=-Djdk.containerized=true" to exclude those tests when testing >> with containers. > > Harold Seigel has updated the pull request incrementally with one additional > commit since the last revision: > > 8238263: Create at-requires mechanism for containers Marked as reviewed by bobv (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/844
Re: RFR: 8248188: Add IntrinsicCandidate and API for Base64 decoding [v8]
On Thu, 22 Oct 2020 22:06:11 GMT, CoreyAshford wrote: >> src/hotspot/cpu/ppc/stubGenerator_ppc.cpp line 3878: >> >>> 3876: // |Element| | | >>> | | | | >>>| | >>> 3877: // >>> +===+=+==+==+=+=+==+==+=+ >>> 3878: // | after vaddubm | 00||b0:0..5 | 00||b0:6..7||b1:0..3 | >>> 00||b1:4..7||b2:0..1 | 00||b2:2..7 | 00||b3:0..5 | 00||b3:6..7||b4:0..3 | >>> 00||b4:4..7||b5:0..1 | 00||b5:2..7 | >> >> An extra line here showing how the 8 6-bit values above get mapping into 6 >> bytes greatly help my brain out. (likewise for the > Just to make sure I understand, you're not asking for a change here, is that > right? I think the first line should also express the initial layout of the 6 bit values similar to the linked algo. I think changing this comment add an extra line which describes the bits as they leave `vaddubm` would be helpful to understand the demangling here. (e.g the `00aa 00bb 00c 00dd` comments in the linked paper) >> src/hotspot/cpu/ppc/stubGenerator_ppc.cpp line 3884: >> >>> 3882: // | vec_0x3fs | 0011 | 0011 | >>> 0011 | 0011 | 0011 | 0011 | >>> 0011 | 0011 | >>> 3883: // >>> +---+-+--+--+-+-+--+--+-+ >>> 3884: // | after vpextd | b5:0..7 | b4:0..7| >>> b3:0..7| b2:0..7 | b1:0..7 | b0:0..7| >>> | | >> >> Are theses comments correct or am I misunderstanding this? I read the final >> result as something starting as `b5:2..7 || b4:4..7|| b5:0..1` from vpextd. > > Because the bytes are displayed e15..e8, instead of the other way around, > it's hard to follow. As an example, consider just the last four bytes of the > table, but displayed in the reverse order: > > 00||b0:0..500||b0:6..7||b1:0..300||b1:4..7||b2:0..100||b2:2..7 > > After vpextd with bit select pattern 0011 for all bytes: > > b0:0..5||b0:6..7b1:0..3||1:4..7b2:0..1||b2:2..7 > = > b0:0..7b1:0..7b2:0..7 > > Should I reverse the order of this table with a comment at the top, to > explain the reason for the reversal? It seems like a good idea. Since you are operating on doublewords here, expressing this as operations on a doubleword instead of bytes would be more intuitive here. I think the lane mappings for little endian are what throw me off. - PR: https://git.openjdk.java.net/jdk/pull/293
Re: RFR: 8254354: Add an asExact() VarHandle combinator [v8]
> Hi, > > This patch adds an asExact() combinator to VarHandle, that will return a new > VarHandle that performs exact type checks, similar to > MethodHandle::invokeExact, to help developers catch inexact VarHandle usage, > which can lead to performance degradation. > > This is implemented using a boolean flag in VarForm. If the flag is set, the > exact type of the invocation is checked against the exact type in the > VarForm. If there is a mismatch, a WrongMethodTypeException is thrown. > > Other than that, there is also an asGeneric() combinator added that does the > inverse operation (thanks to Rémi for the suggestion). I've also added The > `@Hidden` annotation to the VarHandleGuards methods, as well as a > type-checking helper method called from the generic invocation lambda form, > so that the stack trace we get points at the location where the VarHandle is > being used. > > Thanks, > Jorn > > CSR link: https://bugs.openjdk.java.net/browse/JDK-8255375 Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Use AccessType ordinal in guard checks instead of the AccessMode ordinal - Changes: - all: https://git.openjdk.java.net/jdk/pull/843/files - new: https://git.openjdk.java.net/jdk/pull/843/files/16d9fd0e..8e5fb451 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=843=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=843=06-07 Stats: 164 lines in 3 files changed: 0 ins; 0 del; 164 mod Patch: https://git.openjdk.java.net/jdk/pull/843.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/843/head:pull/843 PR: https://git.openjdk.java.net/jdk/pull/843
Re: RFR: 8254354: Add an asExact() VarHandle combinator [v7]
> Hi, > > This patch adds an asExact() combinator to VarHandle, that will return a new > VarHandle that performs exact type checks, similar to > MethodHandle::invokeExact, to help developers catch inexact VarHandle usage, > which can lead to performance degradation. > > This is implemented using a boolean flag in VarForm. If the flag is set, the > exact type of the invocation is checked against the exact type in the > VarForm. If there is a mismatch, a WrongMethodTypeException is thrown. > > Other than that, there is also an asGeneric() combinator added that does the > inverse operation (thanks to Rémi for the suggestion). I've also added The > `@Hidden` annotation to the VarHandleGuards methods, as well as a > type-checking helper method called from the generic invocation lambda form, > so that the stack trace we get points at the location where the VarHandle is > being used. > > Thanks, > Jorn > > CSR link: https://bugs.openjdk.java.net/browse/JDK-8255375 Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Update accessModeType to use the AccessType ordinal directly. - Changes: - all: https://git.openjdk.java.net/jdk/pull/843/files - new: https://git.openjdk.java.net/jdk/pull/843/files/7b5966dd..16d9fd0e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=843=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=843=05-06 Stats: 29 lines in 5 files changed: 7 ins; 3 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/843.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/843/head:pull/843 PR: https://git.openjdk.java.net/jdk/pull/843
Re: RFR: 8254354: Add an asExact() VarHandle combinator
On Fri, 23 Oct 2020 17:47:36 GMT, Jorn Vernee wrote: > Hi, > > This patch adds an asExact() combinator to VarHandle, that will return a new > VarHandle that performs exact type checks, similar to > MethodHandle::invokeExact, to help developers catch inexact VarHandle usage, > which can lead to performance degradation. > > This is implemented using a boolean flag in VarForm. If the flag is set, the > exact type of the invocation is checked against the exact type in the > VarForm. If there is a mismatch, a WrongMethodTypeException is thrown. > > Other than that, there is also an asGeneric() combinator added that does the > inverse operation (thanks to Rémi for the suggestion). I've also added The > `@Hidden` annotation to the VarHandleGuards methods, as well as a > type-checking helper method called from the generic invocation lambda form, > so that the stack trace we get points at the location where the VarHandle is > being used. > > Thanks, > Jorn > > CSR link: https://bugs.openjdk.java.net/browse/JDK-8255375 Looks good to me. - PR: https://git.openjdk.java.net/jdk/pull/843
Re: RFR: 8254354: Add an asExact() VarHandle combinator [v2]
On Mon, 26 Oct 2020 17:13:05 GMT, Rémi Forax
wrote:
>> We can clarify the new methods and tie them closer to method handle
>> semantics. I suggest the names `asExactInvoker` and `asInvoker`, referencing
>> `MethodHandles.exactInvoker` and `MethodHandles.invoker` respectively. (The
>> term "generic" is really reserved for erased method types, and otherwise
>> used internally as the generic invoker.)
>>
>> The `VarHandle` documentation already specifies that:
>>
>> *
>> * Invocation of an access mode method behaves as if an invocation of
>> * {@link MethodHandle#invoke}, where the receiving method handle accepts the
>> * VarHandle instance as the leading argument. More specifically, the
>> * following, where {@code {access-mode}} corresponds to the access mode
>> method
>> * name:
>> * {@code
>> * VarHandle vh = ..
>> * R r = (R) vh.{access-mode}(p1, p2, ..., pN);
>> * }
>> * behaves as if:
>> * {@code
>> * VarHandle vh = ..
>> * VarHandle.AccessMode am =
>> VarHandle.AccessMode.valueFromMethodName("{access-mode}");
>> * MethodHandle mh = MethodHandles.varHandleExactInvoker(
>> * am,
>> * vh.accessModeType(am));
>> *
>> * R r = (R) mh.invoke(vh, p1, p2, ..., pN)
>> * }
>> * (modulo access mode methods do not declare throwing of {@code Throwable}).
>> ...
>>
>> We will need to adjust this section, i can help, but first let us reach
>> agreement on this approach.
>
> Paul,
> an invoker has the MethodHandle (resp. VarHandle) as first argument so it's
> not the same semantics.
I've updated the implementation of accessModeType to work with the ordinal
directly. Note that it was using the AccessType ordinal though, so I also had
to change the parameter type of accessModeTypeUncached, and the respective
implementations, to use AccessType directly (luckily this was possible, since
the current implementations all just used the AccessType).
-
PR: https://git.openjdk.java.net/jdk/pull/843
Re: RFR: 8254354: Add an asExact() VarHandle combinator [v2]
On Mon, 26 Oct 2020 16:34:10 GMT, Paul Sandoz wrote:
>> The direct use of the enum ordinal is because HotSpot accessing it from the
>> enum value is (or was) not optimal in C2.
>>
>> You can avoid the addition of the stable array by doing the following:
>>
>> public final MethodType accessModeType(AccessMode accessMode) {
>> return accessModeType(accessMode.at.ordinal());
>> }
>>
>> @ForceInline
>> final MethodType accessModeType(int accessModeOrdinal) {
>> TypesAndInvokers tis = getTypesAndInvokers();
>> MethodType mt = tis.methodType_table[accessModeOrdinal];
>> if (mt == null) {
>> mt = tis.methodType_table[accessModeOrdinal] =
>> accessModeTypeUncached(accessModeOrdinal);
>> }
>> return mt;
>> }
>>
>> final MethodType accessModeTypeUncached(int accessModeOrdinal) {
>> return
>> accessModeTypeUncached(AccessMode.values()[accessModeOrdinal]);
>> }
>> It's a little odd going back and forth between the ordinal and the enum
>> value, but it's all on the slow uncached path, and it avoids some
>> duplication of code.
>>
>> I'll take a closer looks at the other areas and respond in another comment.
>
> We can clarify the new methods and tie them closer to method handle
> semantics. I suggest the names `asExactInvoker` and `asInvoker`, referencing
> `MethodHandles.exactInvoker` and `MethodHandles.invoker` respectively. (The
> term "generic" is really reserved for erased method types, and otherwise used
> internally as the generic invoker.)
>
> The `VarHandle` documentation already specifies that:
>
> *
> * Invocation of an access mode method behaves as if an invocation of
> * {@link MethodHandle#invoke}, where the receiving method handle accepts the
> * VarHandle instance as the leading argument. More specifically, the
> * following, where {@code {access-mode}} corresponds to the access mode
> method
> * name:
> * {@code
> * VarHandle vh = ..
> * R r = (R) vh.{access-mode}(p1, p2, ..., pN);
> * }
> * behaves as if:
> * {@code
> * VarHandle vh = ..
> * VarHandle.AccessMode am =
> VarHandle.AccessMode.valueFromMethodName("{access-mode}");
> * MethodHandle mh = MethodHandles.varHandleExactInvoker(
> * am,
> * vh.accessModeType(am));
> *
> * R r = (R) mh.invoke(vh, p1, p2, ..., pN)
> * }
> * (modulo access mode methods do not declare throwing of {@code Throwable}).
> ...
>
> We will need to adjust this section, i can help, but first let us reach
> agreement on this approach.
Paul,
an invoker has the MethodHandle (resp. VarHandle) as first argument so it's not
the same semantics.
-
PR: https://git.openjdk.java.net/jdk/pull/843
Re: RFR: 8255206: [macos] LicenseTest fails on macOS 11
On Sat, 24 Oct 2020 00:49:12 GMT, Alexander Matveev wrote: > I did not reproduce issue with license is not shown as per SQE report. It > work as expected. However, jpackage fails due to unflatten/flatten being > removed from hdiutil on macOS 11. I am not sure why it was needed, probably > some legacy code. Without unflatten/flatten everything works as expected on > macOS 11 and 10.5.7. Marked as reviewed by herrick (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/847
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
On Mon, 26 Oct 2020 18:01:04 GMT, John R Rose wrote:
>> @rose00 My bad. You're right, I realized we could also use `asType` later on
>> (I did call it out in the CSR), but It's still a bit more wordy than what
>> I'm proposing.
>>
>> `dropReturn` seemed like a good choice since we already have
>> `dropArguments`. WRT changing to `MethodHandle::changeReturnType`, I think
>> doing that also begs for adding a `MethodHandle::changeParameterType` (with
>> a single index and varargs overload). But, in that case it seems fair to
>> just point users at `asType` instead.
>>
>> In other words; `dropReturn` seems fine to get parity with `dropArguments`,
>> but `changeReturnType` seems a step too close to `asType` (also since it
>> implies adding `changeParameterType` as well). So, maybe this RFE is a bust,
>> and users should use `asType` instead? Or do you think adding both
>> `changeReturnType` and `changeParameterType`(s) seems worth it? (I'm not so
>> sure).
>
>> …`dropReturn` seemed like a good choice since we already have
>> `dropArguments`. WRT changing to `MethodHandle::changeReturnType`...
>
> That's a very reasonable point. People might look for `dropRT` after they
> find `dropAs`. And `MHs` is designed as a large-ish set of utility methods.
>
> I agree that adding `MH::changeRT` is a slippery slope; it tends to lift more
> of the MT API onto MH, and it starts to put new stuff into the smaller MH
> API; that was my bad.
>
> But (in the spirit of brainstorming, and agreeing with your present
> proposal), I'll suggest a separate idea to think about. Use a HOFP API to
> give easy access to the entire `MT` API all in one go, by reifying the
> `MH.type` property as a temporary (lambda argument):
>
> MethodHandle mapType(MethodHandle mh, UnaryOperator fn) {
>return mh.asType(fn.apply(mh.type()));
> }
>
> This also suggests a possible argument transform utility, a sort of
> mini-version of Charlie Nutter's MH builder API:
>
> MethodHandle mapArguments(MethodHandle mh,
> UnaryOperator>> fn) {
> var args = mh.type().argumentList().stream().map(t -> makeArg(t)).asList();
> args = fn.apply(args);
> … do stuff to re-weave mh with the resulting argument transforms …
> }
> public sealed interface ArgumentValue {
> Class type();
>ArgumentValue asType(Class newType);
> … other stuff for pre-applying MHs …
> }
instead of mapArguments(), i believe it's better to have a method map on a
MethodType that takes two mapping functions, one for the return type and one
for the prameter types.
so one can write:
MHs.mapType(mh, mt -> mt.map(Function.identity(), t-> t.isPrimitive? t:
Object.class))
to not change the return type and erase the parameter type to Object if they
are not primitives
With that, changing the return type to void can be done that way too
MHs.mapType(mh, mt -> mt.map(__ -> void.class, Function.identity())
-
PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8238263: Create at-requires mechanism for containers
On Mon, 26 Oct 2020 14:25:14 GMT, Igor Ignatyev wrote: >> Defining an environment variable works when running JTReg from the command >> line. But, mach5 does not pass environment variable settings to its JTReg >> test runs. Some mach5 special command args would still be needed. > >> Defining an environment variable works when running JTReg from the command >> line. But, mach5 does not pass environment variable settings to its JTReg >> test runs. Some mach5 special command args would still be needed. > > right, yet given you also need to explicitly say mach5 that you want to run > testing within docker, that's not a huge problem. this is assuming we default > env. variable to `false`, `make` propagates this env. variable to `jtreg` and > `jtreg` propagates it to the JVM which runs `VMProps` class. Please review this updated webrev that adds environment variable TEST_JDK_CONTAINERIZED for setting @requires jdk.containerized. When TEST_JDK_CONTAINERIZED is unset, jdk.containerized is false. - PR: https://git.openjdk.java.net/jdk/pull/844
Re: RFR: 8238263: Create at-requires mechanism for containers [v2]
> Please review this change to add an @requires mechanism called > "jdk.containerized" to help mark tests that are incompatible with containers. > Users would add "@requires jdk.containerized != true" to the incompatible > tests and then use "make test ... OPTIONS=-Djdk.containerized=true" or "bash > jib.sh mach5 -- remote-build-and-test ... --test-make-args > JTREG=OPTIONS=-Djdk.containerized=true" to exclude those tests when testing > with containers. Harold Seigel has updated the pull request incrementally with one additional commit since the last revision: 8238263: Create at-requires mechanism for containers - Changes: - all: https://git.openjdk.java.net/jdk/pull/844/files - new: https://git.openjdk.java.net/jdk/pull/844/files/9ffe9ad7..7a5ae052 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=844=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=844=00-01 Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/844.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/844/head:pull/844 PR: https://git.openjdk.java.net/jdk/pull/844
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
On Mon, 26 Oct 2020 17:29:11 GMT, Jorn Vernee wrote:
> …`dropReturn` seemed like a good choice since we already have
> `dropArguments`. WRT changing to `MethodHandle::changeReturnType`...
That's a very reasonable point. People might look for `dropRT` after they find
`dropAs`. And `MHs` is designed as a large-ish set of utility methods.
I agree that adding `MH::changeRT` is a slippery slope; it tends to lift more
of the MT API onto MH, and it starts to put new stuff into the smaller MH API;
that was my bad.
But (in the spirit of brainstorming, and agreeing with your present proposal),
I'll suggest a separate idea to think about. Use a HOFP API to give easy
access to the entire `MT` API all in one go, by reifying the `MH.type` property
as a temporary (lambda argument):
MethodHandle mapType(MethodHandle mh, UnaryOperator fn) {
return mh.asType(fn.apply(mh.type()));
}
This also suggests a possible argument transform utility, a sort of
mini-version of Charlie Nutter's MH builder API:
MethodHandle mapArguments(MethodHandle mh,
UnaryOperator>> fn) {
var args = mh.type().argumentList().stream().map(t -> makeArg(t)).asList();
args = fn.apply(args);
… do stuff to re-weave mh with the resulting argument transforms …
}
public sealed interface ArgumentValue {
Class type();
ArgumentValue asType(Class newType);
… other stuff for pre-applying MHs …
}
-
PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
On Mon, 26 Oct 2020 17:16:17 GMT, John R Rose wrote: >> LGTM, pending CSR. >> >> A minor simplification suggested inline. > > I don't mind shorthands, but the existing idiom is shorter than advertised, > a one-liner assuming the MH is already bound to a var: > > target = target.asType(target.type().changeReturnType(void.class)); > > The API has good short circuits already; no new objects are > created if the MH is already void-returning. > > Perhaps this RFE could be adjusted to `MethodHandle::changeReturnType`? > Then it could be used for dropping returns *or* casting them to other types. @rose00 My bad. You're right, I realized we could also use `asType` later on (I did call it out in the CSR), but It's still a bit more wordy than what I'm proposing. `dropReturn` seemed like a good choice since we already have `dropArguments`. WRT changing to `MethodHandle::changeReturnType`, I think doing that also begs for adding a `MethodHandle::changeParameterType` (with a single index and varargs overload). But, in that case it seems fair to just point users at `asType` instead. In other words; `dropReturn` seems fine to get parity with `dropArguments`, but `changeReturnType` seems a step too close to `asType` (also since it implies adding `changeParameterType` as well). So, maybe this RFE is a bust, and users should use `asType` instead? Or do you think adding both `changeReturnType` and `changeParameterType`(s) seems worth it? (I'm not so sure). - PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
On Mon, 26 Oct 2020 15:45:00 GMT, Claes Redestad wrote: >> Jorn Vernee has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Collapse lform get and update into single line >> >> Co-authored-by: Claes Redestad > > LGTM, pending CSR. > > A minor simplification suggested inline. I don't mind shorthands, but the existing idiom is shorter than advertised, a one-liner assuming the MH is already bound to a var: target = target.asType(target.type().changeReturnType(void.class)); The API has good short circuits already; no new objects are created if the MH is already void-returning. Perhaps this RFE could be adjusted to `MethodHandle::changeReturnType`? Then it could be used for dropping returns *or* casting them to other types. - PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8255206: [macos] LicenseTest fails on macOS 11
On Sat, 24 Oct 2020 00:49:12 GMT, Alexander Matveev wrote: > I did not reproduce issue with license is not shown as per SQE report. It > work as expected. However, jpackage fails due to unflatten/flatten being > removed from hdiutil on macOS 11. I am not sure why it was needed, probably > some legacy code. Without unflatten/flatten everything works as expected on > macOS 11 and 10.5.7. Marked as reviewed by asemenyuk (Committer). - PR: https://git.openjdk.java.net/jdk/pull/847
Re: RFR: 8254354: Add an asExact() VarHandle combinator [v2]
On Mon, 26 Oct 2020 16:13:59 GMT, Paul Sandoz wrote:
>> I've updated the javadoc, and added two benchmarks that show the existing
>> discrepancy between an exact and a generic use of a VarHandle, as well as
>> showing that an exact VarHandle is as fast as a generic VarHandle for an
>> exact invocation. (1 benchmark for normal Java field access, and 1 benchmark
>> for the foreign memory-access API).
>>
>> Benchmark Mode
>> Cnt Score Error Units
>> o.o.b.java.lang.invoke.VarHandleExact.exact_exactInvocation avgt
>> 30 0.729 □ 0.010 ns/op
>> o.o.b.java.lang.invoke.VarHandleExact.generic_exactInvocation avgt
>> 30 0.735 □ 0.019 ns/op
>> o.o.b.java.lang.invoke.VarHandleExact.generic_genericInvocation avgt
>> 30 14.696 □ 0.498 ns/op
>> o.o.b.jdk.incubator.foreign.VarHandleExact.exact_exactInvocation avgt
>> 30 2.274 □ 0.012 ns/op
>> o.o.b.jdk.incubator.foreign.VarHandleExact.generic_exactInvocationavgt
>> 30 2.278 □ 0.015 ns/op
>> o.o.b.jdk.incubator.foreign.VarHandleExact.generic_genericInvocation avgt
>> 30 24.759 □ 0.367 ns/op
>
> The direct use of the enum ordinal is because HotSpot accessing it from the
> enum value is (or was) not optimal in C2.
>
> You can avoid the addition of the stable array by doing the following:
>
> public final MethodType accessModeType(AccessMode accessMode) {
> return accessModeType(accessMode.at.ordinal());
> }
>
> @ForceInline
> final MethodType accessModeType(int accessModeOrdinal) {
> TypesAndInvokers tis = getTypesAndInvokers();
> MethodType mt = tis.methodType_table[accessModeOrdinal];
> if (mt == null) {
> mt = tis.methodType_table[accessModeOrdinal] =
> accessModeTypeUncached(accessModeOrdinal);
> }
> return mt;
> }
>
> final MethodType accessModeTypeUncached(int accessModeOrdinal) {
> return accessModeTypeUncached(AccessMode.values()[accessModeOrdinal]);
> }
> It's a little odd going back and forth between the ordinal and the enum
> value, but it's all on the slow uncached path, and it avoids some duplication
> of code.
>
> I'll take a closer looks at the other areas and respond in another comment.
We can clarify the new methods and tie them closer to method handle semantics.
I suggest the names `asExactInvoker` and `asInvoker`, referencing
`MethodHandles.exactInvoker` and `MethodHandles.invoker` respectively. (The
term "generic" is really reserved for erased method types, and otherwise used
internally as the generic invoker.)
The `VarHandle` documentation already specifies that:
*
* Invocation of an access mode method behaves as if an invocation of
* {@link MethodHandle#invoke}, where the receiving method handle accepts the
* VarHandle instance as the leading argument. More specifically, the
* following, where {@code {access-mode}} corresponds to the access mode method
* name:
* {@code
* VarHandle vh = ..
* R r = (R) vh.{access-mode}(p1, p2, ..., pN);
* }
* behaves as if:
* {@code
* VarHandle vh = ..
* VarHandle.AccessMode am =
VarHandle.AccessMode.valueFromMethodName("{access-mode}");
* MethodHandle mh = MethodHandles.varHandleExactInvoker(
* am,
* vh.accessModeType(am));
*
* R r = (R) mh.invoke(vh, p1, p2, ..., pN)
* }
* (modulo access mode methods do not declare throwing of {@code Throwable}).
...
We will need to adjust this section, i can help, but first let us reach
agreement on this approach.
-
PR: https://git.openjdk.java.net/jdk/pull/843
Re: RFR: 8254354: Add an asExact() VarHandle combinator [v2]
On Mon, 26 Oct 2020 13:37:58 GMT, Jorn Vernee wrote:
>> @PaulSandoz I've implemented your suggestion, by moving the `exact` flag to
>> VarHandle itself. FWIW, the VH::accessModeType method took an AccessMode
>> value as an argument, and the AccessDescriptor only stored the ordinal, so I
>> added an `@Stable` array of values to AccessMode to map from ordinal to enum
>> value. But, maybe it makes more sense to just store the AccessMode in the
>> AccessDescriptor directly? (instead of the ordinal). Not sure what the
>> original motivation was for only storing the ordinal.
>>
>> I've also sharpened the tests to check the exception message. Do you think
>> the testing is sufficient? (Note that I did not add tests to the template
>> files since only a select set of argument type conversions causes the WMTE
>> we're looking for. So, that's why I created a new test file).
>>
>> FWIW, there seems to have been a bug in the implementation of
>> IndirectVarHandle::accessModeTypeUncached, where it was using the
>> VarHandle's type as the receiver argument (unlike all the other impls). I've
>> fixed this by passing `null` instead, and also removed a workaround for this
>> problem in VarHandles::maybeAdapt.
>
> I've updated the javadoc, and added two benchmarks that show the existing
> discrepancy between an exact and a generic use of a VarHandle, as well as
> showing that an exact VarHandle is as fast as a generic VarHandle for an
> exact invocation. (1 benchmark for normal Java field access, and 1 benchmark
> for the foreign memory-access API).
>
> Benchmark Mode
> Cnt Score Error Units
> o.o.b.java.lang.invoke.VarHandleExact.exact_exactInvocation avgt
> 30 0.729 □ 0.010 ns/op
> o.o.b.java.lang.invoke.VarHandleExact.generic_exactInvocation avgt
> 30 0.735 □ 0.019 ns/op
> o.o.b.java.lang.invoke.VarHandleExact.generic_genericInvocation avgt
> 30 14.696 □ 0.498 ns/op
> o.o.b.jdk.incubator.foreign.VarHandleExact.exact_exactInvocation avgt
> 30 2.274 □ 0.012 ns/op
> o.o.b.jdk.incubator.foreign.VarHandleExact.generic_exactInvocationavgt
> 30 2.278 □ 0.015 ns/op
> o.o.b.jdk.incubator.foreign.VarHandleExact.generic_genericInvocation avgt
> 30 24.759 □ 0.367 ns/op
The direct use of the enum ordinal is because HotSpot accessing it from the
enum value is (or was) not optimal in C2.
You can avoid the addition of the stable array by doing the following:
public final MethodType accessModeType(AccessMode accessMode) {
return accessModeType(accessMode.at.ordinal());
}
@ForceInline
final MethodType accessModeType(int accessModeOrdinal) {
TypesAndInvokers tis = getTypesAndInvokers();
MethodType mt = tis.methodType_table[accessModeOrdinal];
if (mt == null) {
mt = tis.methodType_table[accessModeOrdinal] =
accessModeTypeUncached(accessModeOrdinal);
}
return mt;
}
final MethodType accessModeTypeUncached(int accessModeOrdinal) {
return accessModeTypeUncached(AccessMode.values()[accessModeOrdinal]);
}
It's a little odd going back and forth between the ordinal and the enum type,
but it's all on the slow uncached path, and it avoids some duplication of code.
I'll take a closer looks at the other areas and respond in another comment.
-
PR: https://git.openjdk.java.net/jdk/pull/843
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
> Hi, > > This patch adds a `dropReturn` combinator to `MethodHandles` that can be used > to create a new method handle that drops the return value, from a given > method handle. Similar to the following code: > > MethodHandle target = ...; > Class targetReturnType = target.type().returnType(); > if (targetReturnType != void.class) > target = filterReturnValue(target, empty(methodType(void.class, > targetReturnType))); > // use target > > But as a short-hand. > > Thanks, > Jorn Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Collapse lform get and update into single line Co-authored-by: Claes Redestad - Changes: - all: https://git.openjdk.java.net/jdk/pull/866/files - new: https://git.openjdk.java.net/jdk/pull/866/files/2793ac1b..e392a0f8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=866=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=866=01-02 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/866.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/866/head:pull/866 PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8251989: Hex formatting and parsing utility [v8]
On Mon, 12 Oct 2020 21:17:47 GMT, Marcono1234
wrote:
>> Roger Riggs has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comment updates to class javadoc
>
> src/java.base/share/classes/java/util/HexFormat.java line 148:
>
>> 146: private static final byte[] LOWERCASE_DIGITS = {
>> 147: '0' , '1' , '2' , '3' , '4' , '5' , '6' , '7',
>> 148: '8' , '9' , 'a' , 'b' , 'c' , 'd' , 'e' , 'f',
>
> Suggestion:
>
> '0', '1', '2', '3', '4', '5', '6', '7',
> '8', '9', 'a', 'b', 'c', 'd', 'e', 'f',
Will remove the extra spaces.
> src/java.base/share/classes/java/util/HexFormat.java line 182:
>
>> 180: /**
>> 181: * Returns a hexadecimal formatter with no delimiter and lowercase
>> characters.
>> 182: * The hex characters are lowercase and the delimiter, prefix, and
>> suffix are empty.
>
>> The hex characters are lowercase
>
> This is already mentioned in the sentence before.
will remove the redundancy.
> src/java.base/share/classes/java/util/HexFormat.java line 195:
>
>> 193:
>> 194: /**
>> 195: * Returns a hexadecimal formatter with a {@code delimiter} and
>> lowercase letters.
>
> Suggestion:
>
> * Returns a hexadecimal formatter with a {@code delimiter} and lowercase
> characters.
> To be consistent with documentation of other methods.
Will correct
-
PR: https://git.openjdk.java.net/jdk/pull/482
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v2]
On Mon, 26 Oct 2020 15:45:34 GMT, Jorn Vernee wrote: >> Hi, >> >> This patch adds a `dropReturn` combinator to `MethodHandles` that can be >> used to create a new method handle that drops the return value, from a given >> method handle. Similar to the following code: >> >> MethodHandle target = ...; >> Class targetReturnType = target.type().returnType(); >> if (targetReturnType != void.class) >> target = filterReturnValue(target, empty(methodType(void.class, >> targetReturnType))); >> // use target >> >> But as a short-hand. >> >> Thanks, >> Jorn > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Fix bug id referenced in test LGTM, pending CSR. A minor simplification suggested inline. src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 5225: > 5223: MethodType newType = oldType.changeReturnType(void.class); > 5224: BoundMethodHandle result = target.rebind(); > 5225: LambdaForm lform = result.form; Suggestion: LambdaForm lform = result.editor().filterReturnForm(V_TYPE, true); src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 5226: > 5224: BoundMethodHandle result = target.rebind(); > 5225: LambdaForm lform = result.form; > 5226: lform = lform.editor().filterReturnForm(V_TYPE, true); Suggestion: - Marked as reviewed by redestad (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v2]
> Hi, > > This patch adds a `dropReturn` combinator to `MethodHandles` that can be used > to create a new method handle that drops the return value, from a given > method handle. Similar to the following code: > > MethodHandle target = ...; > Class targetReturnType = target.type().returnType(); > if (targetReturnType != void.class) > target = filterReturnValue(target, empty(methodType(void.class, > targetReturnType))); > // use target > > But as a short-hand. > > Thanks, > Jorn Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Fix bug id referenced in test - Changes: - all: https://git.openjdk.java.net/jdk/pull/866/files - new: https://git.openjdk.java.net/jdk/pull/866/files/22ae65d0..2793ac1b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=866=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=866=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/866.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/866/head:pull/866 PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator
- Mail original - > De: "Jorn Vernee" > À: "core-libs-dev" > Envoyé: Lundi 26 Octobre 2020 16:37:20 > Objet: RFR: 8255398: Add a dropReturn MethodHandle combinator > Hi, > > This patch adds a `dropReturn` combinator to `MethodHandles` that can be used > to > create a new method handle that drops the return value, from a given method > handle. Similar to the following code: > > MethodHandle target = ...; > Class targetReturnType = target.type().returnType(); > if (targetReturnType != void.class) >target = filterReturnValue(target, empty(methodType(void.class, >targetReturnType))); > // use target > > But as a short-hand. As i said in the JBS issue, it's not just a short hand, it also the semantics of the POP/POP2 bytecode, remove the return value on top of the stack. > > Thanks, > Jorn regards, Rémi > > - > > Commit messages: > - Add a dropReturn method handle combinator > > Changes: https://git.openjdk.java.net/jdk/pull/866/files > Webrev: https://webrevs.openjdk.java.net/?repo=jdk=866=00 > Issue: https://bugs.openjdk.java.net/browse/JDK-8255398 > Stats: 92 lines in 2 files changed: 92 ins; 0 del; 0 mod > Patch: https://git.openjdk.java.net/jdk/pull/866.diff > Fetch: git fetch https://git.openjdk.java.net/jdk pull/866/head:pull/866 > > PR: https://git.openjdk.java.net/jdk/pull/866
RFR: 8255398: Add a dropReturn MethodHandle combinator
Hi, This patch adds a `dropReturn` combinator to `MethodHandles` that can be used to create a new method handle that drops the return value, from a given method handle. Similar to the following code: MethodHandle target = ...; Class targetReturnType = target.type().returnType(); if (targetReturnType != void.class) target = filterReturnValue(target, empty(methodType(void.class, targetReturnType))); // use target But as a short-hand. Thanks, Jorn - Commit messages: - Add a dropReturn method handle combinator Changes: https://git.openjdk.java.net/jdk/pull/866/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=866=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255398 Stats: 92 lines in 2 files changed: 92 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/866.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/866/head:pull/866 PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8238263: Create at-requires mechanism for containers
On Mon, 26 Oct 2020 13:49:26 GMT, Harold Seigel wrote: > Defining an environment variable works when running JTReg from the command > line. But, mach5 does not pass environment variable settings to its JTReg > test runs. Some mach5 special command args would still be needed. right, yet given you also need to explicitly say mach5 that you want to run testing within docker, that's not a huge problem. this is assuming we default env. variable to `false`, `make` propagates this env. variable to `jtreg` and `jtreg` propagates it to the JVM which runs `VMProps` class. - PR: https://git.openjdk.java.net/jdk/pull/844
Re: RFR: 8238263: Create at-requires mechanism for containers
On Fri, 23 Oct 2020 20:08:01 GMT, Igor Ignatyev wrote: >>> I think it depends on whether the tests will be permanently or temporarily >>> excluded from running with containers. I thought this mechanism would be to >>> permanently exclude the tests. That's why I used `@requires`. >> >> I see, if this is for permanent exclusion then yes I agree that `@requires` >> is a better choice. >> enable this option based on an environment variable so we don?t have to remember the >> cryptic command line sequence. >>> I'll look into basing the option on an environment variable. >> >> one will still need to pass an environment variable to jtreg, and hence will >> need to remember some sort of "cryptic command line sequence". a solution >> for that might be to default `jdk.containerized` to `false` in >> `VMProps.java` and when only _containerized_ runs will have to set it up. >> >> >> btw, I'm not sure that `jdk.containerized` is the best name for this >> property as _containerization_ is more of an environmental characteristic >> than that of jdk. how about smth like `env.containerized` or >> `testenv.containerized`? > >> > one will still need to pass an environment variable to jtreg, and hence >> > will need to remember some sort of "cryptic command line sequence". a >> > solution for that might be to default `jdk.containerized` to `false` in >> > `VMProps.java` and when only _containerized_ runs will have to set it up. >> >> Why? Environment variables are inherited. For developers running jtreg, all >> they need to do is export the variable. >> >> % export JAVA_CONTAINERIZED=1 >> % bash >> % echo $JAVA_CONTAINERIZED >> % 1 > > b/c jtreg strips most of the environment variables, they might still be > defined in the process which runs `VMProps` though (I have never checked that) Defining an environment variable works when running JTReg from the command line. But, mach5 does not pass environment variable settings to its JTReg test runs. Some mach5 special command args would still be needed. - PR: https://git.openjdk.java.net/jdk/pull/844
Re: RFR: 8254354: Add an asExact() VarHandle combinator [v2]
On Fri, 23 Oct 2020 23:58:29 GMT, Jorn Vernee wrote:
>> @PaulSandoz Thanks. I initially tested this with memory access VarHandles,
>> which don't erase the receiver type. e.g.
>>
>> MemoryLayout layout = MemoryLayout.ofSequence(10, JAVA_INT);
>> VarHandle vh = layout.varHandle(int.class,
>> MemoryLayout.PathElement.sequenceElement());
>> vh = vh.asExact();
>> try (MemorySegment segment = MemorySegment.allocateNative(layout)) {
>> for (int i = 0; i <10; i++) {
>> vh.set(segment.baseAddress(), i, i);
>> }
>> }
>>
>> Will result in:
>> Exception in thread "main" java.lang.invoke.WrongMethodTypeException:
>> expected (MemoryAddress,long,int)void but found (MemoryAddress,int,int)void
>> at
>> java.base/java.lang.invoke.VarHandleGuards.guard_LII_V(VarHandleGuards.java:915)
>> at main.Main.main(Main.java:18)
>>
>> Which led me to believe the approach would work for other reference types.
>> But, I suppose the MethodTypes fed to memaccess VarForms are non-erased as
>> an exception rather than a rule.
>>
>> I'll update the patch and sharpen the tests to check that the actual
>> expected type is correct (per the exception message).
>
> @PaulSandoz I've implemented your suggestion, by moving the `exact` flag to
> VarHandle itself. FWIW, the VH::accessModeType method took an AccessMode
> value as an argument, and the AccessDescriptor only stored the ordinal, so I
> added an `@Stable` array of values to AccessMode to map from ordinal to enum
> value. But, maybe it makes more sense to just store the AccessMode in the
> AccessDescriptor directly? (instead of the ordinal). Not sure what the
> original motivation was for only storing the ordinal.
>
> I've also sharpened the tests to check the exception message. Do you think
> the testing is sufficient? (Note that I did not add tests to the template
> files since only a select set of argument type conversions causes the WMTE
> we're looking for. So, that's why I created a new test file).
>
> FWIW, there seems to have been a bug in the implementation of
> IndirectVarHandle::accessModeTypeUncached, where it was using the VarHandle's
> type as the receiver argument (unlike all the other impls). I've fixed this
> by passing `null` instead, and also removed a workaround for this problem in
> VarHandles::maybeAdapt.
I've updated the javadoc, and added two benchmarks that show the existing
discrepancy between an exact and a generic use of a VarHandle, as well as
showing that an exact VarHandle is as fast as a generic VarHandle for an exact
invocation. (1 benchmark for normal Java field access, and 1 benchmark for the
foreign memory-access API).
-
PR: https://git.openjdk.java.net/jdk/pull/843
Re: RFR: 8254354: Add an asExact() VarHandle combinator [v6]
> Hi, > > This patch adds an asExact() combinator to VarHandle, that will return a new > VarHandle that performs exact type checks, similar to > MethodHandle::invokeExact, to help developers catch inexact VarHandle usage, > which can lead to performance degradation. > > This is implemented using a boolean flag in VarForm. If the flag is set, the > exact type of the invocation is checked against the exact type in the > VarForm. If there is a mismatch, a WrongMethodTypeException is thrown. > > Other than that, there is also an asGeneric() combinator added that does the > inverse operation (thanks to Rémi for the suggestion). I've also added The > `@Hidden` annotation to the VarHandleGuards methods, as well as a > type-checking helper method called from the generic invocation lambda form, > so that the stack trace we get points at the location where the VarHandle is > being used. > > Thanks, > Jorn Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Add benchmarks - Changes: - all: https://git.openjdk.java.net/jdk/pull/843/files - new: https://git.openjdk.java.net/jdk/pull/843/files/354e233b..7b5966dd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=843=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=843=04-05 Stats: 170 lines in 2 files changed: 170 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/843.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/843/head:pull/843 PR: https://git.openjdk.java.net/jdk/pull/843
Re: RFR: 8255231: Avoid upcalls when initializing the statSampler [v4]
> Current implementation of the statSampler does upcalls to System.getProperty > to collect values for a number of properties that are all provided by the VM > itself. And since the sampling starts before any user code run then no > property can have changed. > > I suggest refactoring the code so that no upcalls are made normally - while > asserting this invariant holds using assert-only upcalls. > > This is a small startup optimization - reducing the startup sequence by > approx. 300k instructions and 70k branches in my linux-x64 setup. Claes Redestad 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 21 additional commits since the last revision: - Address review comments from David Holmes - Merge branch 'master' into com_ns - Refactor to remove stable_java_property_counters and clarify comments - Merge branch 'master' into com_ns - Revert unrelated changes to perfData - Merge branch 'master' into com_ns - Improve comments - typo - Missing definition - Extract the shorthand java.version from VersionProps and use it in StatSampler - ... and 11 more: https://git.openjdk.java.net/jdk/compare/56309bc2...8572159f - Changes: - all: https://git.openjdk.java.net/jdk/pull/802/files - new: https://git.openjdk.java.net/jdk/pull/802/files/6e220227..8572159f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=802=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=802=02-03 Stats: 172 lines in 14 files changed: 99 ins; 38 del; 35 mod Patch: https://git.openjdk.java.net/jdk/pull/802.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/802/head:pull/802 PR: https://git.openjdk.java.net/jdk/pull/802
Re: RFR: 8254354: Add an asExact() VarHandle combinator [v5]
> Hi, > > This patch adds an asExact() combinator to VarHandle, that will return a new > VarHandle that performs exact type checks, similar to > MethodHandle::invokeExact, to help developers catch inexact VarHandle usage, > which can lead to performance degradation. > > This is implemented using a boolean flag in VarForm. If the flag is set, the > exact type of the invocation is checked against the exact type in the > VarForm. If there is a mismatch, a WrongMethodTypeException is thrown. > > Other than that, there is also an asGeneric() combinator added that does the > inverse operation (thanks to Rémi for the suggestion). I've also added The > `@Hidden` annotation to the VarHandleGuards methods, as well as a > type-checking helper method called from the generic invocation lambda form, > so that the stack trace we get points at the location where the VarHandle is > being used. > > Thanks, > Jorn Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: - Update javadoc - Make isExact() public - Changes: - all: https://git.openjdk.java.net/jdk/pull/843/files - new: https://git.openjdk.java.net/jdk/pull/843/files/6d7ad477..354e233b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=843=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=843=03-04 Stats: 26 lines in 1 file changed: 19 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/843.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/843/head:pull/843 PR: https://git.openjdk.java.net/jdk/pull/843
Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v4]
On Fri, 23 Oct 2020 16:21:53 GMT, Jan Lahoda wrote: >> This is an update to javac and javadoc, to introduce support for Preview >> APIs, and generally improve javac and javadoc behavior to more closely >> adhere to JEP 12. >> >> The notable changes are: >> >> * adding support for Preview APIs (javac until now supported primarily only >> preview language features, and APIs associated with preview language >> features). This includes: >> * the @PreviewFeature annotation has boolean attribute "reflective", >> which should be true for reflective Preview APIs, false otherwise. This >> replaces the existing "essentialAPI" attribute with roughly inverted meaning. >> * the preview warnings for preview APIs are auto-suppressed as >> described in the JEP 12. E.g. the module that declares the preview API is >> free to use it without warnings >> * improving error/warning messages. Please see [1] for a list of >> cases/examples. >> * class files are only marked as preview if they are using a preview >> feature. [1] also shows if a class file is marked as preview or not. >> * the PreviewFeature annotation has been moved to jdk.internal.javac >> package (originally was in the jdk.internal package). >> * Preview API in JDK's javadoc no longer needs to specify @preview tag in >> the source files. javadoc will auto-generate a note for @PreviewFeature >> elements, see e.g. [2] and [3] (non-reflective and reflective API, >> respectively). A summary of preview elements is also provided [4]. Existing >> uses of @preview have been updated/removed. >> * non-JDK javadoc is also enhanced to auto-generate preview notes for uses >> of Preview elements, and for declaring elements using preview language >> features [5]. >> >> Please also see the CSR [6] for more information. >> >> [1] >> http://cr.openjdk.java.net/~jlahoda/8250768/JEP12-errors-warnings-v6.html >> [2] >> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.base/java/lang/Record.html >> [3] >> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler/javax/lang/model/element/RecordComponentElement.html >> [4] >> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.html >> [5] http://cr.openjdk.java.net/~jlahoda/8250768/test.javadoc.00/ >> [6] https://bugs.openjdk.java.net/browse/JDK-8250769 > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Removing unnecessary cast. Build changes look good. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/703
Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v12]
On Fri, 23 Oct 2020 14:06:22 GMT, Maurizio Cimadamore wrote: >> Changes requested by ihse (Reviewer). > > @magicus the files you commented on are not part of this PR, but they are > introduced as part of: > https://git.openjdk.java.net/jdk/pull/548 > (you seemed to have approved the changes there - but it's also likely that > this PR doesn't include the latest changes in that PR). Sorry for the > confusion - but please do report any comment you have on the build changes on > that PR! @mcimadamore I'm sorry too for the confusion. :) I must have been a bit in a bit of a hurry when approving it on the other PR. I've now moved my comments there. I don't think there's any way for me to "un-review" this change, so I'll mark it as accepted, even though I don't have anything to say about it (so that I'm not blocking a push). I'll ask the Skara guys if there's a better way to deal with this. Also, in the future, if you are creating a PR which Skara believes has changes in the build system, but it "really" does not, please remove the `build` label, and I won't even see the PR to come bothering you again! ;-) - PR: https://git.openjdk.java.net/jdk/pull/634
Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v12]
On Thu, 22 Oct 2020 17:04:34 GMT, Maurizio Cimadamore wrote: >> 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
Re: RFR: 8254162: Implementation of Foreign-Memory Access API (Third Incubator) [v13]
On Mon, 19 Oct 2020 10:34:32 GMT, Maurizio Cimadamore wrote: >> This patch contains the changes associated with the third incubation round >> of the foreign memory access API incubation (see JEP 393 [1]). This >> iteration focus on improving the usability of the API in 3 main ways: >> >> * first, by providing a way to obtain truly *shared* segments, which can be >> accessed and closed concurrently from multiple threads >> * second, by providing a way to register a memory segment against a >> `Cleaner`, so as to have some (optional) guarantee that the memory will be >> deallocated, eventually >> * third, by not requiring users to dive deep into var handles when they >> first pick up the API; a new `MemoryAccess` class has been added, which >> defines several useful dereference routines; these are really just thin >> wrappers around memory access var handles, but they make the barrier of >> entry for using this API somewhat lower. >> >> A big conceptual shift that comes with this API refresh is that the role of >> `MemorySegment` and `MemoryAddress` is not the same as it used to be; it >> used to be the case that a memory address could (sometimes, not always) have >> a back link to the memory segment which originated it; additionally, memory >> access var handles used `MemoryAddress` as a basic unit of dereference. >> >> This has all changed as per this API refresh; now a `MemoryAddress` is just >> a dumb carrier which wraps a pair of object/long addressing coordinates; >> `MemorySegment` has become the star of the show, as far as dereferencing >> memory is concerned. You cannot dereference memory if you don't have a >> segment. This improves usability in a number of ways - first, it is a lot >> easier to wrap native addresses (`long`, essentially) into a >> `MemoryAddress`; secondly, it is crystal clear what a client has to do in >> order to dereference memory: if a client has a segment, it can use that; >> otherwise, if the client only has an address, it will have to create a >> segment *unsafely* (this can be done by calling >> `MemoryAddress::asSegmentRestricted`). >> >> A list of the API, implementation and test changes is provided below. If >> you have any questions, or need more detailed explanations, I (and the rest >> of the Panama team) will be happy to point at existing discussions, and/or >> to provide the feedback required. >> >> A big thank to Erik Osterlund, Vladimir Ivanov and David Holmes, without >> whom the work on shared memory segment would not have been possible; also >> I'd like to thank Paul Sandoz, whose insights on API design have been very >> helpful in this journey. >> >> Thanks >> Maurizio >> >> Javadoc: >> >> http://cr.openjdk.java.net/~mcimadamore/8254162_v1/javadoc/jdk/incubator/foreign/package-summary.html >> >> Specdiff: >> >> http://cr.openjdk.java.net/~mcimadamore/8254162_v1/specdiff/jdk/incubator/foreign/package-summary.html >> >> CSR: >> >> https://bugs.openjdk.java.net/browse/JDK-8254163 >> >> >> >> ### API Changes >> >> * `MemorySegment` >> * drop factory for restricted segment (this has been moved to >> `MemoryAddress`, see below) >> * added a no-arg factory for a native restricted segment representing >> entire native heap >> * rename `withOwnerThread` to `handoff` >> * add new `share` method, to create shared segments >> * add new `registerCleaner` method, to register a segment against a cleaner >> * add more helpers to create arrays from a segment e.g. `toIntArray` >> * add some `asSlice` overloads (to make up for the fact that now segments >> are more frequently used as cursors) >> * rename `baseAddress` to `address` (so that `MemorySegment` can implement >> `Addressable`) >> * `MemoryAddress` >> * drop `segment` accessor >> * drop `rebase` method and replace it with `segmentOffset` which returns >> the offset (a `long`) of this address relative to a given segment >> * `MemoryAccess` >> * New class supporting several static dereference helpers; the helpers are >> organized by carrier and access mode, where a carrier is one of the usual >> suspect (a Java primitive, minus `boolean`); the access mode can be simple >> (e.g. access base address of given segment), or indexed, in which case the >> accessor takes a segment and either a low-level byte offset,or a high level >> logical index. The classification is reflected in the naming scheme (e.g. >> `getByte` vs. `getByteAtOffset` vs `getByteAtIndex`). >> * `MemoryHandles` >> * drop `withOffset` combinator >> * drop `withStride` combinator >> * the basic memory access handle factory now returns a var handle which >> takes a `MemorySegment` and a `long` - from which it is easy to derive all >> the other handles using plain var handle combinators. >> * `Addressable` >> * This is a new interface which is attached to entities which can be >> projected to a `MemoryAddress`. For now, both `MemoryAddress` and >>
Re: RFR: 8255331: Problemlist java/foreign/TestMismatch.java on 32-bit platforms until JDK-8254162
On Fri, 23 Oct 2020 10:42:14 GMT, Aleksey Shipilev wrote: > I would like to have clean x86_32 test runs until JDK-8254162 arrives. > > Testing: > - [x] Affected test on Linux x86_64 (still passes) > - [x] Affected test on Linux x86_32 (now ignored) @mcimadamore, please take a look? This seems to be the only thing that keeps `x86_32` `tier1` from being clean. I think you'd need to remove this test from the problem list with/after JDK-8254162 integration. - PR: https://git.openjdk.java.net/jdk/pull/833
Integrated: 8255343: java/util/stream/SpliteratorTest.java fails on 32-bit platforms with "Misaligned access at address: 12"
On Fri, 23 Oct 2020 12:37:10 GMT, Aleksey Shipilev wrote: > It currently fails on x86_32 with `java.lang.IllegalStateException: > Misaligned access at address: 12`. I checked that once JDK-8254162 > integrates, that issue is gone. Until then, having clean x86_32 testing is > beneficial for other work. > > Testing: > - [x] `java/util/stream` tests on Linux x86_64 (still passes) > - [x] `java/util/stream` tests on Linux x86_32 (start to pass) This pull request has now been integrated. Changeset: c28b0111 Author:Aleksey Shipilev URL: https://git.openjdk.java.net/jdk/commit/c28b0111 Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod 8255343: java/util/stream/SpliteratorTest.java fails on 32-bit platforms with "Misaligned access at address: 12" Co-authored-by: Maurizio Cimadamore Reviewed-by: mcimadamore - PR: https://git.openjdk.java.net/jdk/pull/836
