Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive
On Wed, 16 Sep 2020 19:05:56 GMT, Ioi Lam wrote:
>> This patch is reorganized after 8252725, which is separated from this patch
>> to refactor jlink glugin code. The previous
>> webrev with hg can be found at:
>> http://cr.openjdk.java.net/~minqi/2020/8247536/webrev-05. With 8252725
>> integrated, the
>> regeneration of holder classes is simply to call the new added
>> GenerateJLIClassesHelper.cdsGenerateHolderClasses
>> function. Tests: tier1-4
>
> Changes requested by iklam (Reviewer).
> _Mailing list message from [Mandy Chung](mailto:mandy.ch...@oracle.com) on
> [build-dev](mailto:build-...@openjdk.java.net):_
> src/java.base/share/classes/java/lang/invoke/GenerateJLIClassesHelper.java
> 367 /** 368 * called from vm to generate MethodHandle holder classes 369
> * @return @code { Object[] } if holder classes can be generated. 370 *
> @param lines the output lines from @code { VM.cdsTraceResolve } 371 */
> @code {} is wrong syntax. It should be {@code } 372 static
> Object[] cdsGenerateHolderClasses(String[] lines) { this can be made
> private as it's invoked by VM only. Can you move it to the end of the
Will change access to 'private'
> file. 373 try { 374 Map result =
> generateHolderClasses(Arrays.stream(lines)); 375 if (result == null) {
> 376 return null; 377 }
>
> generateHolderClasses should never return null and so line 371-377 can
> be dropped. ? I also suggest to add in the generateHolderClasses method
> to add Objects.requireNonNull(traces).
>
Will drop the check and add Objects.requireNonNull(traces).
> 379 Object[] ret_array = new Object[size * 2];
>
> Rename `ret_array` to retArray to follow the naming convention using camel
> case.
>
> src/java.base/share/classes/jdk/internal/misc/VM.java
> 457 isDumpLoadedClassListSetAndOpen = isDumpLoadedClassListSetAndOpen0();
>
> This should be cached in the caller who checks if -XX:+DumpLoadedClassList
> instead of every VM initialization.
>
> Since there are many CDS-related methods, I think it's time to introduce
> a new CDS class for these methods to reside (maybe jdk.internal.vm.CDS?).
>
> I suggest to add CDS:logTraceResolve(String line) method that
> will check if isDumpLoadedClassListSetAndOpen is true, then call the
> native cdsTraceResolve (or whatever name). This way,
> GenerateJLIClassesHelper can simply call CDS::logTraceResolve.
>
Created a separate issue https://bugs.openjdk.java.net/browse/JDK-8253208 to
move CDS method to CDS.java
All CDS related code will be added to CDS.java
> 493 * Output to DumpLoadedClassList, format is simimar to LF_RESOLVE
>
> s/simimar/similar
>
> 494 * @see InvokerBytecodeGenerator
> 495 * @param line the line to output.
>
> @see is typically placed after @param.
>
> Should it say @see java.lang.invoke.GenerateJLIClassesHelper::traceLambdaForm
> instead of InvokerBytecodeGenerator?
>
Right, will change that to the suggestion.
> Mandy
>
> On 9/15/20 12:15 PM, Yumin Qi wrote:
-
PR: https://git.openjdk.java.net/jdk/pull/193
Re: 'Find' method for Iterable
On 9/16/20 1:59 PM, Remi Forax wrote:
- Mail original -
De: "Nir Lisker"
À: "core-libs-dev"
Envoyé: Lundi 14 Septembre 2020 20:56:27
Objet: 'Find' method for Iterable
Hi,
This has probably been brought up at some point. When we need to find an
item in a collection based on its properties, we can either do it in a
loop, testing each item, or in a stream with filter and findFirst/Any.
I would think that a method in Iterable be useful, along the lines of:
public Optional find(Predicate condition) {
Objects.requireNonNull(condition);
for (T t : this) {
if (condition.test(t)) {
return Optional.of(t);
}
}
return Optional.empty();
}
With usage:
list.find(person -> person.id == 123456);
There are a few issues with the method here such as t being null in
null-friendly collections and the lack of bound generic types, but this
example is just used to explain the intention.
It will be an alternative to
list.stream().filter(person -> person.id == 123456).findAny/First()
(depending on if the collection is ordered or not)
which doesn't create a stream, similar to Iterable#forEach vs
Stream#forEach.
Maybe with pattern matching this would become more appetizing.
During the development of Java 8, we first tried to use Iterator/Iterable
instead of using a novel interface Stream.
But a Stream cleanly separate the lazy side effect free API from the mutable
one (Collection) and can be optimized better by the VM (it's a push API instead
of being a pull API).
The other question is why there is no method find() on Collection, i believe
it's because while find() is ok for any DB API, find() is dangerous on a
Collection because the execution time is linear, so people may use it instead
of using a Map.
Hi Nir,
Rémi is correct to point out this distinction between the lazy operations (which
appear on Stream) and the eager (and possibly mutating) operations on Collections. I
think we want to preserve this distinction.
While it might not be difficult to add a find() method to Iterable, why limit it to
the find operation, and what about all the other operations available on Stream?
Maybe what's necessary is a way to convert an Iterable to a Stream. In fact, this is
already possible:
StreamSupport.stream(iterable.spliterator(), false)
Well, this is mouthful, so maybe there ought to be an easier way to convert an
Iterable to a Stream.
On the other hand, your examples use a list. The List interface already has methods
indexOf/lastIndexOf which search the list for a particular object that's compared
using equals(). It seems reasonable to consider similar methods that take a
predicate instead of an object.
Does either of these sound promising?
s'marks
Re: RFR: 8253240: No javadoc for DecimalFormatSymbols.hashCode() [v2]
On Wed, 16 Sep 2020 19:53:26 GMT, Roger Riggs wrote: >> Looks in line with the discussion on core-libs. > > Removing hashcode from the serialized form needs a CSR because it was > non-transient in JDK 15. > The intent is to not use the serialized value, so the readObject method > should zero it out > so it can be computed on first use. Drafted a CSR for this PR. Please review. - PR: https://git.openjdk.java.net/jdk/pull/208
Re: 'Find' method for Iterable
- Mail original -
> De: "Nir Lisker"
> À: "core-libs-dev"
> Envoyé: Lundi 14 Septembre 2020 20:56:27
> Objet: 'Find' method for Iterable
> Hi,
>
> This has probably been brought up at some point. When we need to find an
> item in a collection based on its properties, we can either do it in a
> loop, testing each item, or in a stream with filter and findFirst/Any.
>
> I would think that a method in Iterable be useful, along the lines of:
>
> public Optional find(Predicate condition) {
>Objects.requireNonNull(condition);
>for (T t : this) {
> if (condition.test(t)) {
> return Optional.of(t);
>}
>}
>return Optional.empty();
> }
>
> With usage:
>
> list.find(person -> person.id == 123456);
>
> There are a few issues with the method here such as t being null in
> null-friendly collections and the lack of bound generic types, but this
> example is just used to explain the intention.
>
> It will be an alternative to
>
> list.stream().filter(person -> person.id == 123456).findAny/First()
> (depending on if the collection is ordered or not)
>
> which doesn't create a stream, similar to Iterable#forEach vs
> Stream#forEach.
>
> Maybe with pattern matching this would become more appetizing.
During the development of Java 8, we first tried to use Iterator/Iterable
instead of using a novel interface Stream.
But a Stream cleanly separate the lazy side effect free API from the mutable
one (Collection) and can be optimized better by the VM (it's a push API instead
of being a pull API).
The other question is why there is no method find() on Collection, i believe
it's because while find() is ok for any DB API, find() is dangerous on a
Collection because the execution time is linear, so people may use it instead
of using a Map.
>
> - Nir
Rémi
Re: RFR: 8253240: No javadoc for DecimalFormatSymbols.hashCode() [v2]
On Wed, 16 Sep 2020 17:51:18 GMT, Lance Andersen wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Addressing Roger's comments. > > Looks in line with the discussion on core-libs. Removing hashcode from the serialized form needs a CSR because it was non-transient in JDK 15. The intent is to not use the serialized value, so the readObject method should zero it out so it can be computed on first use. - PR: https://git.openjdk.java.net/jdk/pull/208
Re: RFR: JDK-8230652: Improve verbose output
On Sat, 12 Sep 2020 18:30:08 GMT, Andy Herrick wrote: > JDK-8230652 > Extracting the commands displayed by verbose output (including commands > called thru ToolProvider) , to contain the the > command, it's output, and it's return value on separate lines and formatted > in a way that they can be easily cut and > pasted into a script. Marked as reviewed by asemenyuk (Committer). - PR: https://git.openjdk.java.net/jdk/pull/141
Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive
On Tue, 15 Sep 2020 18:57:55 GMT, Yumin Qi wrote:
> This patch is reorganized after 8252725, which is separated from this patch
> to refactor jlink glugin code. The previous
> webrev with hg can be found at:
> http://cr.openjdk.java.net/~minqi/2020/8247536/webrev-05. With 8252725
> integrated, the
> regeneration of holder classes is simply to call the new added
> GenerateJLIClassesHelper.cdsGenerateHolderClasses
> function. Tests: tier1-4
Changes requested by iklam (Reviewer).
src/hotspot/share/classfile/lambdaFormInvokers.cpp line 51:
> 49: #include "runtime/os.hpp"
> 50: #include "runtime/signature.hpp"
> 51:
Are all these header files needed? E.g., typeArrayKlass.hpp doesn't seem to be
needed.
src/hotspot/share/classfile/lambdaFormInvokers.cpp line 121:
> 119: log_info(cds)("Class %s not present, skip", name);
> 120: return;
> 121: }
Since the classlist can be generated by the user, it may cause the assert at
line 115 to fail. For example, no
java.lang.invoke.*$Holder classes are used by HelloWorld: $ java -verbose
-Xshare:off -cp . HelloWorld | grep Holder
[0.030s][info][class,load] java.util.KeyValueHolder source: jrt:/java.base
[0.080s][info][class,load] java.security.SecureClassLoader$DebugHolder source:
jrt:/java.base
$
But it's possible for the user to generate a classlist using HelloWorld, and
then manually add LF_RESOLVE lines into
the classlist.
So I think line 114 should be changed to a regular lookup (the symbol is
created if it doesn't exist), and line 115
should be removed.
Also, we should add some test cases for invalid LF_RESOLVE input. You can see
examples in
[appcds/customLoader/ClassListFormatA.java](https://github.com/openjdk/jdk/blob/d250f9e08c4a0c047cd3e619918c116f568e31d4/test/hotspot/jtreg/runtime/cds/appcds/customLoader/ClassListFormatA.java#L51).
Since the new tests aren't related to custom loader, we should probably move
[appcds/customLoader/ClassListFormatBase.java](https://github.com/openjdk/jdk/blob/d250f9e08c4a0c047cd3e619918c116f568e31d4/test/hotspot/jtreg/runtime/cds/appcds/customLoader/ClassListFormatBase.java#L30)
under appcds/, and add a new file like appcds/ClassListFormat.java
src/hotspot/share/classfile/lambdaFormInvokers.cpp line 158:
> 156: // find_class assert on SystemDictionary_lock or safepoint
> 157: MutexLocker lock(SystemDictionary_lock);
> 158: InstanceKlass* old = SystemDictionary::find_class(class_name, cld);
There's no need to call `find_class` here, since it will return the same class
as `klass` on line 117.
src/hotspot/share/classfile/lambdaFormInvokers.hpp line 27:
> 25: #ifndef SHARE_MEMORY_LAMBDAFORMINVOKERS_HPP
> 26: #define SHARE_MEMORY_LAMBDAFORMINVOKERS_HPP
> 27: #include "memory/allocation.hpp"
For the AllStatic base class, you should use memory/allStatic.hpp instead.
src/hotspot/share/classfile/systemDictionary.cpp line 1875:
> 1873: VerifyDuringStartup ||
> 1874: VerifyAfterGC ||
> 1875: DumpSharedSpaces, "too expensive");
This may not be needed if you remove the find_class() call from
LambdaFormInvokers::regenerate_holder_classes?
src/java.base/share/classes/java/lang/invoke/GenerateJLIClassesHelper.java line
67:
> 65: if (VM.isDumpLoadedClassListSetAndOpen) {
> 66: VM.cdsTraceResolve(traceLF);
> 67: }
GenerateJLIClassesHelper shouldn't need to know why the trace is needed. Also,
"cdsTraceResolve" is too generic.
I think it's better to have
if (TRACE_RESOLVE || VM.CDS_TRACE_JLINV_RESOLVE) {
...
VM.cdsTraceJLINVResolve(traceLF);
The acronym JLINV is used in
[methodHandles.cpp](https://github.com/openjdk/jdk/blob/ce93cbce77e1f4baa52676826c8ae27d474360b6/src/hotspot/share/prims/methodHandles.cpp#L1524)
src/java.base/share/classes/jdk/internal/misc/VM.java line 490:
> 488: */
> 489: public static boolean isDumpLoadedClassListSetAndOpen;
> 490: private static native boolean isDumpLoadedClassListSetAndOpen0();
I would suggest to rename to `isDumpingLoadedClassList`
-
PR: https://git.openjdk.java.net/jdk/pull/193
Re: RFR: 8252730: jlink does not create reproducible builds on different servers [v6]
> Related to [JDK-8252730 jlink does not create reproducible builds on different > servers](https://bugs.openjdk.java.net/browse/JDK-8252730). Introduces > ordering based on `Archive` module names to > ensure stable files (and file signatures) across generated JDK images by > jlink. Ian Graves has updated the pull request incrementally with two additional commits since the last revision: - Merging two tests and renaming a test - Merging tests and renaming test 4 - Changes: - all: https://git.openjdk.java.net/jdk/pull/156/files - new: https://git.openjdk.java.net/jdk/pull/156/files/3e8aaf2d..1af70014 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=156=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=156=04-05 Stats: 210 lines in 3 files changed: 78 ins; 124 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/156.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/156/head:pull/156 PR: https://git.openjdk.java.net/jdk/pull/156
Re: RFR: 8252730: jlink does not create reproducible builds on different servers [v5]
> Related to [JDK-8252730 jlink does not create reproducible builds on different > servers](https://bugs.openjdk.java.net/browse/JDK-8252730). Introduces > ordering based on `Archive` module names to > ensure stable files (and file signatures) across generated JDK images by > jlink. Ian Graves has updated the pull request incrementally with one additional commit since the last revision: Copying test jdk to test consistency - Changes: - all: https://git.openjdk.java.net/jdk/pull/156/files - new: https://git.openjdk.java.net/jdk/pull/156/files/860c6096..3e8aaf2d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=156=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=156=03-04 Stats: 122 lines in 1 file changed: 122 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/156.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/156/head:pull/156 PR: https://git.openjdk.java.net/jdk/pull/156
Re: RFR: 8253240: No javadoc for DecimalFormatSymbols.hashCode() [v2]
> Hi, > > Please review the fix to the issue wrt missing hashCode() javadoc, which was > recently discussed in core-libs ml. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Addressing Roger's comments. - Changes: - all: https://git.openjdk.java.net/jdk/pull/208/files - new: https://git.openjdk.java.net/jdk/pull/208/files/3276598e..2f9d0c3a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=208=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=208=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/208.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/208/head:pull/208 PR: https://git.openjdk.java.net/jdk/pull/208
Re: RFR: 8253240: No javadoc for DecimalFormatSymbols.hashCode() [v2]
On Wed, 16 Sep 2020 17:47:17 GMT, Roger Riggs wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Addressing Roger's comments. > > src/java.base/share/classes/java/text/DecimalFormatSymbols.java line 1153: > >> 1151: * Cached hash code >> 1152: */ >> 1153: private volatile int hashCode; > > I'm thinking the cached hashcode should be / should have been transient (not > serialized). > And add a period at the end of the sentence above. Good point. Addressed both comments. - PR: https://git.openjdk.java.net/jdk/pull/208
Re: RFR: 8253240: No javadoc for DecimalFormatSymbols.hashCode()
On Wed, 16 Sep 2020 17:29:52 GMT, Naoto Sato wrote: > Hi, > > Please review the fix to the issue wrt missing hashCode() javadoc, which was > recently discussed in core-libs ml. Looks in line with the discussion on core-libs. - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/208
Re: RFR: 8253240: No javadoc for DecimalFormatSymbols.hashCode()
On Wed, 16 Sep 2020 17:29:52 GMT, Naoto Sato wrote: > Hi, > > Please review the fix to the issue wrt missing hashCode() javadoc, which was > recently discussed in core-libs ml. Changes requested by rriggs (Reviewer). src/java.base/share/classes/java/text/DecimalFormatSymbols.java line 1153: > 1151: * Cached hash code > 1152: */ > 1153: private volatile int hashCode; I'm thinking the cached hashcode should be / should have been transient (not serialized). And add a period at the end of the sentence above. - PR: https://git.openjdk.java.net/jdk/pull/208
RFR: 8253240: No javadoc for DecimalFormatSymbols.hashCode()
Hi, Please review the fix to the issue wrt missing hashCode() javadoc, which was recently discussed in core-libs ml. - Commit messages: - 8253240: No javadoc for DecimalFormatSymbols.hashCode() Changes: https://git.openjdk.java.net/jdk/pull/208/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=208=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8253240 Stats: 6 lines in 1 file changed: 5 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/208.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/208/head:pull/208 PR: https://git.openjdk.java.net/jdk/pull/208
Re: RFR: 8252730: jlink does not create reproducible builds on different servers [v3]
On Tue, 15 Sep 2020 14:30:03 GMT, Alan Bateman wrote: >> Ian Graves has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Comparator cleanup > > The update using flatMap looks good. I think we need to explore adding more > tests if possible. Thanks for adding a test. What would you think about adding this to JLinkReproducible2Test to avoid duplication. The existing test can be tweaked to do the mismatch check with different sets of modules. In passing, no need to include java.base in the @modules tag. - PR: https://git.openjdk.java.net/jdk/pull/156
Integrated: 8244706: GZIP "OS" header flag hard-coded to 0 instead of 255 (RFC 1952 non-compliance)
On Fri, 11 Sep 2020 14:17:45 GMT, Jaikiran Pai wrote: > Can I please get a review and a sponsor for this patch which fixes the issue > reported in > https://bugs.openjdk.java.net/browse/JDK-8244706? > The commit here sets the `OS` header flag to `255` (which represents > `unknown`) as noted in [1]. A new test has been > included in this commit to verify the change. Furthermore, this doesn't > impact the `java.util.zip.GZIPInputStream` > since it ignores [2] this header value while reading the headers from the > stream. [1] > https://tools.ietf.org/html/rfc1952#page-7 [2] > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#L173 This pull request has now been integrated. Changeset: e5866aa7 Author:Jaikiran Pai Committer: Lance Andersen URL: https://git.openjdk.java.net/jdk/commit/e5866aa7 Stats: 79 lines in 2 files changed: 0 ins; 77 del; 2 mod 8244706: GZIP "OS" header flag hard-coded to 0 instead of 255 (RFC 1952 non-compliance) Reviewed-by: lancea, bchristi - PR: https://git.openjdk.java.net/jdk/pull/130
Re: [BUG] Java 15: DecimalFormatSymbols overrides equals but not hashCode
I created a JIRA issue for this:
https://bugs.openjdk.java.net/browse/JDK-8253240
Naoto
On 9/16/20 2:11 AM, Pavel Rappo wrote:
On 15 Sep 2020, at 21:50, Rob Spoor wrote:
On 15/09/2020 22:02, Pavel Rappo wrote:
On 15 Sep 2020, at 20:50, Brian Burkhalter wrote:
On Sep 15, 2020, at 12:38 PM, Kevin Rushforth
wrote:
I see this in DecimalFormatSymbols:
/**
* Override hashCode.
*/
private volatile int hashCode;
@Override
public int hashCode() {
Although, I'm not sure why the intervening private field would prevent javadoc
from generating at least a method with an empty doc
https://bugs.openjdk.java.net/browse/JDK-8187386
So in this case, the solution would be to remove the superfluous "Override
equals." comment from the equals method, right?
It depends on what you want to achieve. On the one hand, if you 'remove the superfluous
"Override equals."' it will eliminate the discrepancy that you described in
your original email. On the other hand, a reader might no longer have a clear indication
that equals and hashCode are meaningfully overridden.
I would simply return "/** Override hashCode */" to its original place, as it
is clearly a bug that affects documentation.
-Pavel
Re: RFR: 8251397: NPE on ClassValue.ClassValueMap.cacheArray [v2]
On Wed, 9 Sep 2020 12:53:41 GMT, Galder Zamarreño wrote: >> Ah yes, my bad 臘♂️ > > Updated PR. @galderz did you really update the PR? I still see the _storeFence_ before the write. - PR: https://git.openjdk.java.net/jdk/pull/94
Re: RFR: 8244706: GZIP "OS" header flag hard-coded to 0 instead of 255 (RFC 1952 non-compliance) [v4]
On Tue, 15 Sep 2020 16:54:29 GMT, Lance Andersen wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove unintended file commit > > Thank you for making the changes that Brent and I suggested. This looks > good. I have submitted, a > [CSR](https://bugs.openjdk.java.net/browse/JDK-8253142) to track the change. > I have also created a [Release > Note](https://bugs.openjdk.java.net/browse/JDK-8253185) as well. > > Once the CSR has been approved, I will sponsor your change Thank you Lance for taking care of the CSR and the release notes. Given that the CSR has been approved, I'm initating the integrate command. - PR: https://git.openjdk.java.net/jdk/pull/130
Re: [BUG] Java 15: DecimalFormatSymbols overrides equals but not hashCode
> On 15 Sep 2020, at 21:50, Rob Spoor wrote:
>
> On 15/09/2020 22:02, Pavel Rappo wrote:
>>> On 15 Sep 2020, at 20:50, Brian Burkhalter
>>> wrote:
>>>
On Sep 15, 2020, at 12:38 PM, Kevin Rushforth
wrote:
I see this in DecimalFormatSymbols:
/**
* Override hashCode.
*/
>>> private volatile int hashCode;
@Override
public int hashCode() {
Although, I'm not sure why the intervening private field would prevent
javadoc from generating at least a method with an empty doc
>> https://bugs.openjdk.java.net/browse/JDK-8187386
>
> So in this case, the solution would be to remove the superfluous "Override
> equals." comment from the equals method, right?
It depends on what you want to achieve. On the one hand, if you 'remove the
superfluous "Override equals."' it will eliminate the discrepancy that you
described in your original email. On the other hand, a reader might no longer
have a clear indication that equals and hashCode are meaningfully overridden.
I would simply return "/** Override hashCode */" to its original place, as it
is clearly a bug that affects documentation.
-Pavel
Re: RFR: 8250859: Address reliance on default constructors in the Accessibility APIs
On Tue, 15 Sep 2020 19:40:28 GMT, Phil Race wrote: >> This issue relates to JDK-8250639 '☂ Address reliance on default >> constructors in the java.desktop module'. The >> following classes have had an explicit no-arg constructor added, with a >> protected access modifier and accompanying API >> description: >> - Default ctor on com.sun.java.accessibility.util.SwingEventMonitor >> - Default ctor on javax.accessibility.AccessibleContext >> - Default ctor on javax.accessibility.AccessibleHyperlink >> >> The following classes have had an explicit no-arg constructor added, with a >> public access modifier and accompanying API >> description: >> - Default ctor on javax.accessibility.AccessibleResourceBundle >> - Default ctor on com.sun.java.accessibility.util.AWTEventMonitor >> - Default ctor on com.sun.java.accessibility.util.AccessibilityEventMonitor >> - Default ctor on com.sun.java.accessibility.util.AccessibilityListenerList >> - Default ctor on com.sun.java.accessibility.util.EventID >> >> specdiff: >> http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250859/webrevs/webrev.00/specdiff/overview-summary.html >> bug: >> https://bugs.openjdk.java.net/browse/JDK-8250859 > > This looks OK but the CSR needs updates first. The CSR lists `com.sun.java.accessibility.util.SwingEventMonitor` as being changed, but I cannot find that class in this PR. - PR: https://git.openjdk.java.net/jdk/pull/174
Re: RFR: 8202473: A type variable with multiple bounds does not correctly place type annotation [v2]
On Tue, 15 Sep 2020 15:18:20 GMT, Rafael Winterhalter wrote: >> This patch assures that an annotation on a type bound is placed on the >> correct bound index. > > Rafael Winterhalter 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 jfranck (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/155
Re: RFR: 8241390: 'Deadlock' with VM_RedefineClasses::lock_classes()
On Tue, 15 Sep 2020 20:02:38 GMT, Coleen Phillimore wrote: >> The change fixes a 'deadlock' situation in >> VM_RedefineClasses::lock_classes() when the current thread is in the middle >> of redefining the same class. The change is based on the fix [1] suggested >> in the Jira issue [2] comment. >> [1] http://cr.openjdk.java.net/~jiangli/8241390/webrev.00/ >> [2] https://bugs.openjdk.java.net/browse/JDK-8241390 >> >> Testing: :jdk_instrument, tier1-tier3, and tier5 tests pass. > > Changes requested by coleenp (Reviewer). The identifier 'cls' does not reflect the nature of the object and needs to be replaces with something like: classes, redef_classes or classes_list This impacts the files: jvmtiThreadState.hpp and jvmtiRedefineClasses.cpp. - PR: https://git.openjdk.java.net/jdk/pull/190
Re: RFR: 8241390: 'Deadlock' with VM_RedefineClasses::lock_classes()
On Tue, 15 Sep 2020 20:02:33 GMT, Coleen Phillimore wrote:
>> The change fixes a 'deadlock' situation in
>> VM_RedefineClasses::lock_classes() when the current thread is in the middle
>> of redefining the same class. The change is based on the fix [1] suggested
>> in the Jira issue [2] comment.
>> [1] http://cr.openjdk.java.net/~jiangli/8241390/webrev.00/
>> [2] https://bugs.openjdk.java.net/browse/JDK-8241390
>>
>> Testing: :jdk_instrument, tier1-tier3, and tier5 tests pass.
>
> src/hotspot/share/prims/jvmtiRedefineClasses.cpp line 159:
>
>> 157: if (!cls->contains(def_ik)) {
>> 158: def_ik->set_is_being_redefined(false);
>> 159: }
>
> Ok, so adding the Klass to the thread local list for each recursion works
> like ref counting. Can't think of a simpler
> way to do this. Looks good.
Yes, the same class can be pushed to the list multiple times (not more than
once by each recursive redefinition). It'd
make sense to add a comment about it as it is not obvious.
> test/jdk/java/lang/instrument/MakeAgentJAR.sh line 1:
>
>> 1: #!/bin/sh
>
> There are tests in test/hotspot/jtreg/serviceability/jvmti/RedefineClasses
> that don't use shell scripts that are much
> better. Can you add this test using that framework instead?
I'm second for this suggestion from Coleen to get rid of the shell script in
the test.
-
PR: https://git.openjdk.java.net/jdk/pull/190
