Re: RFR: 8255883: Avoid multiple GeneratedMethodAccessor for same NativeMethod… [v6]
On Fri, 13 Nov 2020 16:02:42 GMT, Jorn Vernee wrote: >> Thanks all! May this test-request get approved? > > Notice that the `/test` command is currently un-available (some > implementation concerns are still under consideration). As an alternative, > GitHub actions can be used to do basic automatic testing when pushes occur. > > However, pushes to the master branch are ignored. The tests didn't run for > this PR since it was created from the `master` branch instead of a feature > branch (this is ill-advised > https://github.com/openjdk/jdk/pull/1070#issuecomment-722108502). > > @huishi-hs My advice for now would be to push the changes for this PR to a > separate new branch in your fork, for which the GitHub actions workflow > should run, and then simply include a link to the workflow here (if you want > to share the results). according to instructions, test is performed with another PR and passed https://github.com/openjdk/jdk/pull/1213 - PR: https://git.openjdk.java.net/jdk/pull/1070
Re: RFR: 8255883: Avoid duplicated GeneratedMethodAccessor when reflect method invoked from different threads
On Sat, 14 Nov 2020 00:28:58 GMT, Jorn Vernee wrote: >> @JornVernee >> Could you please help approve and start tier1 test? >> This is same PR with https://github.com/openjdk/jdk/pull/1070 > > @huishi-hs Testing should start automatically, and will appear here when you > push to any branch in your fork that is not `master`: > https://github.com/huishi-hs/jdk/actions (unless you disabled this in the > repo settings). > > As I said, the `/test` command does not work at this time. @JornVernee Thanks! I didn't enable https://github.com/huishi-hs/jdk/actions. Now it works. - PR: https://git.openjdk.java.net/jdk/pull/1213
RFR: 8256318: AArch64: Add support for floating-point absolute difference
This supports for floating-point absolute difference instructions, i.e. FABD scalar/vector. Verified with linux-aarch64-server-release, tier1-3. Added a JMH micro `test/micro/org/openjdk/bench/vm/compiler/FloatingScalarVectorAbsDiff.java` for performance test. The FABD (scalar), the performance tests handle registers directly, the average latency reduces to almost half (~57%) of the original. For FABD (vector), we restrict the data size (~24KB) to be less than L1 data cache size (32KB), so that the memory access can hit in L1, and witness 14.2% (float) and 21.2% (double) improvements. The JMH results on Kunpeng916: Benchmark(count) (seed) Mode Cnt ScoreError Units # before, fsub+fabs FloatingScalarVectorAbsDiff.testScalarAbsDiffDouble 1024 316731 avgt 10 6038.333 ± 3.889 ns/op FloatingScalarVectorAbsDiff.testScalarAbsDiffFloat 1024 316731 avgt 10 6005.125 ± 3.025 ns/op FloatingScalarVectorAbsDiff.testVectorAbsDiffDouble 1024 316731 avgt 10 950.340 ± 9.398 ns/op FloatingScalarVectorAbsDiff.testVectorAbsDiffFloat 1024 316731 avgt 10 454.350 ± 1.798 ns/op # after, fabd FloatingScalarVectorAbsDiff.testScalarAbsDiffDouble 1024 316731 avgt 10 3483.801 ± 1.763 ns/op FloatingScalarVectorAbsDiff.testScalarAbsDiffFloat 1024 316731 avgt 10 3442.412 ± 1.866 ns/op FloatingScalarVectorAbsDiff.testVectorAbsDiffDouble 1024 316731 avgt 10 816.301 ± 4.454 ns/op FloatingScalarVectorAbsDiff.testVectorAbsDiffFloat 1024 316731 avgt 10 354.710 ± 1.001 ns/op - Commit messages: - 8256318: AArch64: Add support for floating-point absolute difference Changes: https://git.openjdk.java.net/jdk/pull/1215/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1215=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8256318 Stats: 209 lines in 20 files changed: 176 ins; 0 del; 33 mod Patch: https://git.openjdk.java.net/jdk/pull/1215.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1215/head:pull/1215 PR: https://git.openjdk.java.net/jdk/pull/1215
Re: RFR: 8255947: [macos] Signed macOS jpackage app doesn't filter spurious '-psn' argument [v2]
On Fri, 13 Nov 2020 00:47:19 GMT, Alexander Matveev
wrote:
>> This is regression from JDK-8242302 and for some reason removing -psn
>> argument code was removed during refactoring. Fixed be adding removing -psn
>> argument back. Also, test was added to test this functionality.
>
> Alexander Matveev has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8255947: [macos] Signed macOS jpackage app doesn't filter spurious '-psn'
> argument [v2]
test/jdk/tools/jpackage/macosx/ArgumentsFilteringTest.java line 52:
> 50: public class ArgumentsFilteringTest {
> 51:
> 52: public static void main(String[] args) throws Exception {
Please don't use main() and TKit.run(). Use @Test annotation for methods with
test cases.
Please put each case in separated class method.
You can use BasicTest class as a reference.
-
PR: https://git.openjdk.java.net/jdk/pull/1154
Re: RFR: 8255883: Avoid duplicated GeneratedMethodAccessor when reflect method invoked from different threads
On Fri, 13 Nov 2020 23:56:21 GMT, Hui Shi wrote: >> …ructorAccessorImpl object >> >> duplicate with https://github.com/openjdk/jdk/pull/1070, try trigger test >> with new PR > > @JornVernee > Could you please help approve and start tier1 test? > This is same PR with https://github.com/openjdk/jdk/pull/1070 @huishi-hs Testing should start automatically, and will appear here when you push to any branch in your fork that is not `master`: https://github.com/huishi-hs/jdk/actions (unless you disabled this in the repo settings). As I said, the `/test` command does not work at this time. - PR: https://git.openjdk.java.net/jdk/pull/1213
Re: RFR: 8255883: Avoid duplicated GeneratedMethodAccessor when reflect method invoked from different threads
On Fri, 13 Nov 2020 23:37:46 GMT, Hui Shi wrote: > …ructorAccessorImpl object > > duplicate with https://github.com/openjdk/jdk/pull/1070, try trigger test > with new PR @JornVernee Could you please help approve and start tier1 test? This is same PR with https://github.com/openjdk/jdk/pull/1070 - PR: https://git.openjdk.java.net/jdk/pull/1213
RFR: 8255883: Avoid duplicated GeneratedMethodAccessor when reflect method invoked from different threads
…ructorAccessorImpl object duplicate with https://github.com/openjdk/jdk/pull/1070, try trigger test with new PR - Commit messages: - 8255883: Avoid multiple GeneratedAccessor for same NativeMethod/ConstructorAccessorImpl object Changes: https://git.openjdk.java.net/jdk/pull/1213/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1213=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255883 Stats: 46 lines in 2 files changed: 28 ins; 0 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/1213.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1213/head:pull/1213 PR: https://git.openjdk.java.net/jdk/pull/1213
Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs [v3]
On Fri, 13 Nov 2020 18:20:37 GMT, Kevin Rushforth wrote: >> Andy Herrick has updated the pull request incrementally with one additional >> commit since the last revision: >> >> JDK-8189198: Add "forRemoval = true" to Applet APIs > > src/java.desktop/share/classes/java/applet/package-info.java line 40: > >> 38: * >> 39: * Deprecated. >> 40: * This package has been deprecated and may be removed in a future >> version of the Java Platform. > > That should be `@deprecated This package ...`. See > [java/rmi/activation/package-info.java#L41](https://github.com/openjdk/jdk/blob/master/src/java.rmi/share/classes/java/rmi/activation/package-info.java#L41). The deprecation description should point to the new API which might be used instead of the deprecated ones. So the text "deprecated without replacement" was intentionally added, it will be good to preserve it. - PR: https://git.openjdk.java.net/jdk/pull/1127
Re: RFR: 8253459: Formatter treats index, width and precision > Integer.MAX_VALUE incorrectly [v8]
> 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: Moving additional methods to package private - Changes: - all: https://git.openjdk.java.net/jdk/pull/516/files - new: https://git.openjdk.java.net/jdk/pull/516/files/4bcb053e..5a0effe1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=516=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=516=06-07 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 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: RFR: 8253459: Formatter treats index, width and precision > Integer.MAX_VALUE incorrectly [v7]
On Tue, 6 Oct 2020 19:10:46 GMT, Ian Graves wrote: >> Is the new exception type useful? yes, it matches the previous pattern. >> But it does not (and none of the IllegalFormatException subclasses) produce >> a readable message with the offending value. So the developer will not see >> anything useful. >> The fine grained exceptions provide little value. > > I've been on the fence about this, personally. The Formatter uses pretty > fine-grained exception types for error conditions. I'd be okay discontinuing > this practice here, but am not sure what to replace this with. Perhaps we > enable `IllegalFormatException` to be, itself, public and instantiable ? Updates (including cleaning up some git weirdness with rebasing) include adherence to the new CSR draft proposal. This makes the new exception type package-private. - PR: https://git.openjdk.java.net/jdk/pull/516
Re: RFR: 8253459: Formatter treats index, width and precision > Integer.MAX_VALUE incorrectly [v7]
> 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 with a new target base due to a merge or a rebase. The pull request now contains seven commits: - Making IllegalFormatArgumentIndexException package private - Additional specificity around width - Tweaking verbiage - Updating docs specifying exception for 0 indices - Throwing exceptions for zeroth indexes - Updating docs - Updating docs and throwing errors accordingly - Changes: https://git.openjdk.java.net/jdk/pull/516/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=516=06 Stats: 94 lines in 4 files changed: 86 ins; 0 del; 8 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: RFR: 8253459: Formatter treats index, width and precision > Integer.MAX_VALUE incorrectly [v6]
> 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 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 14 additional commits since the last revision: - Merge branch 'format-string-numeric-bound' of github.com:igraves/jdk into format-string-numeric-bound - Additional specificity around width - Tweaking verbiage - Updating docs specifying exception for 0 indices - Throwing exceptions for zeroth indexes - Updating docs - Updating docs and throwing errors accordingly - Making IllegalFormatArgumentIndexException package private - Additional specificity around width - Tweaking verbiage - ... and 4 more: https://git.openjdk.java.net/jdk/compare/96b6dcaf...a84d3f2f - Changes: - all: https://git.openjdk.java.net/jdk/pull/516/files - new: https://git.openjdk.java.net/jdk/pull/516/files/0526ef43..a84d3f2f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=516=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=516=04-05 Stats: 302178 lines in 580 files changed: 296317 ins; 3588 del; 2273 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
Feature Request: CharSequence.getChars
Hi all,
In Java 9, compact strings were added. Ever since, I've been trying to
use getChars instead of charAt for Strings. However, I prefer to also do
the same for StringBuilder and StringBuffer. This can lead to some
special cases. For instance, in Apache Commons I/O:
https://github.com/apache/commons-io/blob/master/src/main/java/org/apache/commons/io/input/CharSequenceReader.java#L232
Is it perhaps not a good idea to add getChars as a default method to
CharSequence? The implementation is simple enough:
default void getChars(int srcBegin, int srcEnd, char[] dst, int
dstBegin) {
Objects.checkFromToIndex(srcBegin, srcEnd, length());
int n = srcEnd - srcBegin;
Objects.checkFromIndexSize(dstBegin, n, dst.length);
for (int i = srcBegin; i < srcEnd; i++) {
dst[dstBegin++] = charAt(i);
}
}
String, StringBuilder and StringBuffer automatically override it with
their own methods, so these won't have to change at all.
I've already worked a bit on this, including more efficient overrides
for CharBuffer and StringCharBuffer. The only other known
implementation, Segment, won't profit much from a custom implementation,
so I skipped it.
Should I continue with my work, or is it something that's considered not
necessary? Adding it would allow a better implementation of
Writer.append. For large CharSequences, that's currently hopelessly
inefficient - first it converts the CharSequence to String, then to
char[]. Instead, it could be implemented like write(String, int, int),
with only one conversion (CharSequence to char[]).
Kind regards,
Rob
Re: Asking to contribute(?)
It appears this discussion has died out...
I really think it's a great addition to have a transform method added to
not just Stream*, but also StringBuilder, StringBuffer and Optional. A
search for classes containing "Builder" shows the following that could
also be interesting:
* HttpClient.Builder, HttpRequest.Builder and WebSocket.Builder
* DateTimeFormatterBuilder
* ProcessBuilder
If we don't go for the Transformable interface (which I admit looks iffy
due to the cast that can fail if a class decides to use a different
generic type), I think we should at least add a transform method to
Stream, StringBuilder, StringBuffer and Optional. The other classes I
mentioned are probably not used as often as these 4, so let's omit them
to keep the scope of the change limited.
* For the same reasons that OptionalInt, OptionalLong and OptionalDouble
won't get (extra) map methods, let's skip IntStream, LongStream and
DoubleStream as well.
On 05/11/2020 06:23, Justin Dekeyser wrote:
Hello,
Thank you for your answer, I appreciate!
Indeed, it is clear to me that if the feature should be a concern of both
String and Stream (or more?), a common contract can be designed.
The impl. you sketch is the more natural one I think. (it's also the one I
gave in the other mailing grap about that toList stuff, so I guess it's ok!)
I am just a bit cold about the idea that the spec will make the compiler's
job, but I guess in Java there is no work around.
I don't know what the community thinks about it.
Regards,
Justin Dekeyser
On Wednesday, November 4, 2020, Rob Spoor wrote:
On 04/11/2020 14:18, Justin Dekeyser wrote:
Hello everyone,
I have been following this mailing list for several months, and
earlier today my attention was drawn to
https://bugs.openjdk.java.net/browse/JDK-8140283. Actually I've been
dreaming of such a feature for a long time now.
I would really be interested in solving it, but I do not know its
current state nor if someone would agree to sponsor my work on that.
It would be my very first intervention in the Java code base.
(Still have to make sure the Oracle agreement paper does not conflict
with my current job contract, so nothing's ready for now.)
Thank you for your time,
Best regards,
Justin Dekeyser
I'd like this feature as well, but why stop at Stream? String already has
the transform method, but StringBuilder (and StringBuffer) could also use
it.
And that's where you're likely to start copy pasting. I've done so for
several builder classes I've written for myself. So here's a thought: why
add this method to classes, when you can create a trait using an interface
with a default method?
public interface Transformable {
default R transform(Function f) {
// note: this would need documentation that a class X is
// only allowed to implement Transformable
return f.apply((T) this);
}
}
So you could get the following, and each would automatically get the
transform method:
* public class String implements Transformable
* public class StringBuilder implements Transformable
* public class StringBuffer implements Transformable
* public interface Stream implements Transformable>
RFC: 8256341: Add bootstrap methods for getting array elements
(Continuing this thread from relevant discussion on [1]) Discussing the following RFE: https://bugs.openjdk.java.net/browse/JDK-8256341 Also (as mentioned offline), while it's possible with the current API to provide an `Object[]` as class data, and then load elements of that array into CP entries using downstream condys (e.g. by creating an array access var handle and then using that), I think it would be good to add a `getArrayElement` BSM for doing that to `ConstantBootstraps` to make this (seemingly) common case easier (maybe also a `getField` for loading instance fields). This would help to address the case where multiple live constants need to be injected. I am uncomfortable with adding `ConstantBootstraps::getArrayElement` because an array is modifiable and _not_ true constant. A final instance field can be modified via reflection and therefore it's not trusted as a constant either. I guess it would be similar to doing something like: private static final Object[] arr = getArray(); private static final Object o = arr[0]; Setting `arr[0] = new Object();` would not affect `o` after it has been initialized. The difference being that a constant for the array element would be resolved (more) lazily, so it would be possible to resolve the array, then modify it, and then resolve the constant that loads the element, which would see the updated value as well. However, we can already store an array in the constant pool today, and modify it if we want, even though, as you say, it is mutable. In that sense getArrayElement doesn't introduce anything new. Mutating the array is probably a bad idea though, so maybe we want to push users away from using arrays? To prevent inadvertent modification, maybe an immutable `List` should be used in place of a plain array. In that case, would you feel differently about added a `getListElement` BSM, that gets an element from an (immutable) List instead? Another idea is to rely on records, since they can not be mutated with reflection at all. i.e. we add a getRecordComponent BSM instead, so a class data Object can be a record, and then the BSM can be used to extract individual components. Though, the record class would probably have to be reflectively accessible from the point where the constant is resolved as well. WDYT? Jorn [1] : https://github.com/openjdk/jdk/pull/1171
Re: RFR: 8230501: Class data support for hidden classes [v2]
On Fri, 13 Nov 2020 18:42:53 GMT, Mandy Chung wrote: > > Also (as mentioned offline), while it's possible with the current API to > > provide an `Object[]` as class data, and then load elements of that array > > into CP entries using downstream condys (e.g. by creating an array access > > var handle and then using that), I think it would be good to add a > > `getArrayElement` BSM for doing that to `ConstantBootstraps` to make this > > (seemingly) common case easier (maybe also a `getField` for loading > > instance fields). This would help to address the case where multiple live > > constants need to be injected. > > I am uncomfortable with adding `ConstantBootstraps::getArrayElement` because > an array is modifiable and _not_ true constant. A final instance field can be > modified via reflection and therefore it's not trusted as a constant either. I guess it would be similar to doing something like: private static final Object[] arr = getArray(); private static final Object o = arr[0]; Setting `arr[0] = new Object();` would not affect `o` after it has been initialized. The difference being that a constant for the array element would be resolved (more) lazily, so it would be possible to resolve the array, then modify it, and then resolve the constant that loads the element, which would see the updated value as well. However, we can already store an array in the constant pool today, and modify it if we want, even though, as you say, it is mutable. In that sense getArrayElement doesn't introduce anything new. Mutating the array is probably a bad idea though, so maybe we want to push users away from using arrays? To prevent inadvertent modification, maybe an immutable `List` should be used in place of a plain array. In that case, would you feel differently about added a `getListElement` BSM, that gets an element from an (immutable) List instead? Another idea is to rely on records, since they can not be mutated with reflection at all. i.e. we add a getRecordComponent BSM instead, so a class data Object can be a record, and then the BSM can be used to extract individual components. WDYT? - PR: https://git.openjdk.java.net/jdk/pull/1171
Re: RFR: 8230501: Class data support for hidden classes [v2]
On Fri, 13 Nov 2020 19:23:54 GMT, Jorn Vernee wrote: >>> Also (as mentioned offline), while it's possible with the current API to >>> provide an `Object[]` as class data, and then load elements of that array >>> into CP entries using downstream condys (e.g. by creating an array access >>> var handle and then using that), I think it would be good to add a >>> `getArrayElement` BSM for doing that to `ConstantBootstraps` to make this >>> (seemingly) common case easier (maybe also a `getField` for loading >>> instance fields). This would help to address the case where multiple live >>> constants need to be injected. >> >> I am uncomfortable with adding `ConstantBootstraps::getArrayElement` because >> an array is modifiable and _not_ true constant. A final instance field can >> be modified via reflection and therefore it's not trusted as a constant >> either. > >> > Also (as mentioned offline), while it's possible with the current API to >> > provide an `Object[]` as class data, and then load elements of that array >> > into CP entries using downstream condys (e.g. by creating an array access >> > var handle and then using that), I think it would be good to add a >> > `getArrayElement` BSM for doing that to `ConstantBootstraps` to make this >> > (seemingly) common case easier (maybe also a `getField` for loading >> > instance fields). This would help to address the case where multiple live >> > constants need to be injected. >> >> I am uncomfortable with adding `ConstantBootstraps::getArrayElement` because >> an array is modifiable and _not_ true constant. A final instance field can >> be modified via reflection and therefore it's not trusted as a constant >> either. > > I guess it would be similar to doing something like: > > private static final Object[] arr = getArray(); > private static final Object o = arr[0]; > > Setting `arr[0] = new Object();` would not affect `o` after it has been > initialized. The difference being that a constant for the array element would > be resolved (more) lazily, so it would be possible to resolve the array, then > modify it, and then resolve the constant that loads the element, which would > see the updated value as well. > > However, we can already store an array in the constant pool today, and modify > it if we want, even though, as you say, it is mutable. In that sense > getArrayElement doesn't introduce anything new. > > Mutating the array is probably a bad idea though, so maybe we want to push > users away from using arrays? To prevent inadvertent modification, maybe an > immutable `List` should be used in place of a plain array. In that case, > would you feel differently about added a `getListElement` BSM, that gets an > element from an (immutable) List instead? > > Another idea is to rely on records, since they can not be mutated with > reflection at all. i.e. we add a getRecordComponent BSM instead, so a class > data Object can be a record, and then the BSM can be used to extract > individual components. > > WDYT? Sorry, this is unrelated to this RFR, I will start a separate discussion thread elsewhere. - PR: https://git.openjdk.java.net/jdk/pull/1171
Re: RFR: 8230501: Class data support for hidden classes [v2]
On Thu, 12 Nov 2020 15:19:30 GMT, Jorn Vernee wrote: > Also (as mentioned offline), while it's possible with the current API to > provide an `Object[]` as class data, and then load elements of that array > into CP entries using downstream condys (e.g. by creating an array access var > handle and then using that), I think it would be good to add a > `getArrayElement` BSM for doing that to `ConstantBootstraps` to make this > (seemingly) common case easier (maybe also a `getField` for loading instance > fields). This would help to address the case where multiple live constants > need to be injected. I am uncomfortable with adding `ConstantBootstraps::getArrayElement` because an array is modifiable and _not_ true constant. A final instance field can be modified via reflection and therefore it's not trusted as a constant either. - PR: https://git.openjdk.java.net/jdk/pull/1171
Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs [v3]
On Fri, 13 Nov 2020 15:05:15 GMT, Andy Herrick wrote: >> JDK-8189198: Add "forRemoval = true" to Applet APIs > > Andy Herrick has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8189198: Add "forRemoval = true" to Applet APIs src/java.desktop/share/classes/java/applet/package-info.java line 40: > 38: * > 39: * Deprecated. > 40: * This package has been deprecated and may be removed in a future > version of the Java Platform. That should be `@deprecated This package ...`. See [java/rmi/activation/package-info.java#L41](https://github.com/openjdk/jdk/blob/master/src/java.rmi/share/classes/java/rmi/activation/package-info.java#L41). - PR: https://git.openjdk.java.net/jdk/pull/1127
Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs [v2]
On Fri, 13 Nov 2020 18:01:18 GMT, Kevin Rushforth wrote: >> src/java.naming/share/classes/javax/naming/Context.java line 1087: >> >>> 1085: @Deprecated(since="16", forRemoval=true) >>> 1086: String APPLET = "java.naming.applet"; >>> 1087: }; >> >> Probably should be since="9" (the deprecation in JDK-8051422 pre-dates the >> enhanced deprecation work). > > Good point, since it was in fact deprecated in 9. yes - changed to since="9" this morning - PR: https://git.openjdk.java.net/jdk/pull/1127
Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs [v2]
On Fri, 13 Nov 2020 09:31:53 GMT, Alan Bateman wrote: >> Andy Herrick 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 six additional >> commits since the last revision: >> >> - JDK-8189198: Add "forRemoval = true" to Applet APIs >> - Merge branch 'master' into JDK-8189198 >> - Merge branch 'master' into JDK-8189198 >> - JDK-8189198: Add "forRemoval = true" to Applet APIs >> - JDK-8189198: Add "forRemoval = true" to Applet APIs >> - JDK-8189198: Add "forRemoval = true" to Applet APIs > > src/java.naming/share/classes/javax/naming/Context.java line 1087: > >> 1085: @Deprecated(since="16", forRemoval=true) >> 1086: String APPLET = "java.naming.applet"; >> 1087: }; > > Probably should be since="9" (the deprecation in JDK-8051422 pre-dates the > enhanced deprecation work). Good point, since it was in fact deprecated in 9. - PR: https://git.openjdk.java.net/jdk/pull/1127
Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v23]
> 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
Re: RFR: 8255883: Avoid multiple GeneratedMethodAccessor for same NativeMethod… [v6]
On Fri, 13 Nov 2020 14:01:28 GMT, Hui Shi wrote: >> Thanks for the update, latest version looks good. > > Thanks all! May this test-request get approved? Notice that the `/test` command is currently un-available (some implementation concerns are still under consideration). As an alternative, GitHub actions can be used to do basic automatic testing when pushes occur. However, pushes to the master branch are ignored. The tests didn't run for this PR since it was created from the `master` branch instead of a feature branch (this is ill-advised https://github.com/openjdk/jdk/pull/1070#issuecomment-722108502). @huishi-hs My advice for now would be to push the changes for this PR to a separate new branch in your fork, for which the GitHub actions workflow should run, and then simply include a link to the workflow here (if you want to share the results). - PR: https://git.openjdk.java.net/jdk/pull/1070
Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs [v3]
> JDK-8189198: Add "forRemoval = true" to Applet APIs Andy Herrick has updated the pull request incrementally with one additional commit since the last revision: JDK-8189198: Add "forRemoval = true" to Applet APIs - Changes: - all: https://git.openjdk.java.net/jdk/pull/1127/files - new: https://git.openjdk.java.net/jdk/pull/1127/files/d9850cd8..c6ea7714 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1127=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1127=01-02 Stats: 6 lines in 2 files changed: 4 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1127.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1127/head:pull/1127 PR: https://git.openjdk.java.net/jdk/pull/1127
Re: RFR: 8255883: Avoid multiple GeneratedMethodAccessor for same NativeMethod… [v6]
On Fri, 13 Nov 2020 08:33:30 GMT, Alan Bateman wrote: >> Hui Shi has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. The pull request contains one >> new commit since the last revision: >> >> 8255883: Avoid multiple GeneratedAccessor for same >> NativeMethod/ConstructorAccessorImpl object > > Thanks for the update, latest version looks good. Thanks all! May this test-request get approved? - PR: https://git.openjdk.java.net/jdk/pull/1070
Re: RFR: 8255947: [macos] Signed macOS jpackage app doesn't filter spurious '-psn' argument [v2]
On Fri, 13 Nov 2020 00:47:19 GMT, Alexander Matveev wrote: >> This is regression from JDK-8242302 and for some reason removing -psn >> argument code was removed during refactoring. Fixed be adding removing -psn >> argument back. Also, test was added to test this functionality. > > Alexander Matveev has updated the pull request incrementally with one > additional commit since the last revision: > > 8255947: [macos] Signed macOS jpackage app doesn't filter spurious '-psn' > argument [v2] Looks good - Marked as reviewed by herrick (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1154
Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v22]
> 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
Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v21]
> 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: 8255964: Add all details to jstack log in jtreg timeout handler
On Thu, 5 Nov 2020 17:09:58 GMT, Nils Eliasson wrote: > This patch adds jcmd Thread.print to the jtreg timeout handler. > > Please review. This pull request has now been integrated. Changeset: 41139e31 Author:Nils Eliasson URL: https://git.openjdk.java.net/jdk/commit/41139e31 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8255964: Add all details to jstack log in jtreg timeout handler Reviewed-by: iignatyev - PR: https://git.openjdk.java.net/jdk/pull/1080
Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs [v2]
On Thu, 12 Nov 2020 20:48:13 GMT, Andy Herrick wrote: >> JDK-8189198: Add "forRemoval = true" to Applet APIs > > Andy Herrick 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 six additional > commits since the last revision: > > - JDK-8189198: Add "forRemoval = true" to Applet APIs > - Merge branch 'master' into JDK-8189198 > - Merge branch 'master' into JDK-8189198 > - JDK-8189198: Add "forRemoval = true" to Applet APIs > - JDK-8189198: Add "forRemoval = true" to Applet APIs > - JDK-8189198: Add "forRemoval = true" to Applet APIs src/java.naming/share/classes/javax/naming/Context.java line 1087: > 1085: @Deprecated(since="16", forRemoval=true) > 1086: String APPLET = "java.naming.applet"; > 1087: }; Probably should be since="9" (the deprecation in JDK-8051422 pre-dates the enhanced deprecation work). - PR: https://git.openjdk.java.net/jdk/pull/1127
Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v6]
> 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 24 commits: - 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 - non x86_64 arch support - c2 test case - intrinsic - ... and 14 more: https://git.openjdk.java.net/jdk/compare/b4d01867...90493e6e - Changes: https://git.openjdk.java.net/jdk/pull/1003/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1003=05 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
Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v5]
> 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 incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 23 additional commits since the last revision: - 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 - non x86_64 arch support - c2 test case - intrinsic - Use overloads of method names. Simplify internally to avoid overload resolution issues, leverging List for the exception mapper. - ... and 13 more: https://git.openjdk.java.net/jdk/compare/a9aed82d...e3887a79 - Changes: - all: https://git.openjdk.java.net/jdk/pull/1003/files - new: https://git.openjdk.java.net/jdk/pull/1003/files/692b4298..e3887a79 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1003=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1003=03-04 Stats: 48430 lines in 396 files changed: 26604 ins; 14405 del; 7421 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
Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v3]
On Sat, 7 Nov 2020 11:38:50 GMT, Vladimir Ivanov wrote: >> Roland Westrelin has refreshed the contents of this pull request, and >> previous commits have been removed. The incremental views will show >> differences compared to the previous content of the PR. > > Marked as reviewed by vlivanov (Reviewer). I noticed the compiler test would fail with -Xcomp. I pushed a fix for that - PR: https://git.openjdk.java.net/jdk/pull/1003
Re: RFR: 8255883: Avoid multiple GeneratedMethodAccessor for same NativeMethod… [v6]
On Fri, 13 Nov 2020 03:50:09 GMT, Hui Shi wrote: >> …AccessorImpl object >> >> We met real problem when using protobuf with option optimized for code size, >> detail in JBS https://bugs.openjdk.java.net/browse/JDK-8255883 >> >> Optimize solution is adding a new boolean field to detect concurrent method >> accessor generation in same NativeMethodAccessorImpl object, only one thread >> is allowed to generate accessor, other threads still invoke in jni way until >> parent's delegator is updated from NativeMethodAccessorImpl to generated >> accessor. >> >> In common case, extra overhead is an atomic operation, compared with method >> accessor generate, this cost is trivial. > > Hui Shi has refreshed the contents of this pull request, and previous commits > have been removed. The incremental views will show differences compared to > the previous content of the PR. The pull request contains one new commit > since the last revision: > > 8255883: Avoid multiple GeneratedAccessor for same > NativeMethod/ConstructorAccessorImpl object Thanks for the update, latest version looks good. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1070
