Integrated: 8269685: Optimize HeapHprofBinWriter implementation
On Fri, 2 Jul 2021 12:05:35 GMT, Lin Zang wrote: > This PR rewrite the implementation of the HeapHprofBinWriter, which could > simplify the logic of current implementation. > please see detail description at > https://bugs.openjdk.java.net/browse/JDK-8269685. This pull request has now been integrated. Changeset: 8876eae4 Author:Lin Zang URL: https://git.openjdk.java.net/jdk/commit/8876eae42993d4425ba9886dde94b08f6101a257 Stats: 500 lines in 3 files changed: 156 ins; 318 del; 26 mod 8269685: Optimize HeapHprofBinWriter implementation 8262386: resourcehogs/serviceability/sa/TestHeapDumpForLargeArray.java timed out Reviewed-by: sspitsyn, amenkov - PR: https://git.openjdk.java.net/jdk/pull/4666
RFR: 8274398: Suppress more warnings on non-serializable non-transient instance fields in management libs
Follow-up change to JDK-8232442, augmentations to javac's Xlint:serial checking are out for review (#5709) and various management libraries would need some changes to pass under the expanded checks. The changes are to suppress warnings where non-transient fields in serializable types are not declared with a type statically known to be serializable. That isn't necessarily a correctness issues, but it does merit further scrutiny. I'll run a script to update copyright years before a push. - Commit messages: - 8274398: Suppress more warnings on non-serializable non-transient instance fields in management libs Changes: https://git.openjdk.java.net/jdk/pull/5726/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5726=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274398 Stats: 12 lines in 8 files changed: 12 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5726.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5726/head:pull/5726 PR: https://git.openjdk.java.net/jdk/pull/5726
Re: RFR: 8274318: Replace 'for' cycles with iterator with enhanced-for in java.management
On Sat, 25 Sep 2021 10:51:12 GMT, Andrey Turbanov wrote: > There are a few places in code, where manual `for` loop is used with Iterator > to iterate over Collection. > Instead of manual `for` cycles, it's preferred to use enhanced-for cycle > instead: it's less verbose, makes code easier to read and it's less > error-prone. > It doesn't have any performance impact: javac compiler generates similar code > when compiling enhanced-for cycle. > Similar cleanups: > 1. [JDK-8274016](https://bugs.openjdk.java.net/browse/JDK-8274016) > java.desktop > 2. [JDK-8274237](https://bugs.openjdk.java.net/browse/JDK-8274237) java.base > 3. [JDK-8274261](https://bugs.openjdk.java.net/browse/JDK-8274261) jdk.jcmd Marked as reviewed by cjplummer (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5695
Re: RFR: 8274261: Use enhanced-for instead of plain 'for' in jdk.jcmd
On Fri, 24 Sep 2021 07:30:02 GMT, Andrey Turbanov wrote: > There are few places in code where manual `for` loop is used with Iterator to > iterate over Collection or Array. > Instead of manual `for` cycles it's preferred to use enhanced-for cycle > instead: it's less verbose, makes code easier to read and it's less > error-prone. > It doesn't have any performance impact: javac compiler generates similar code > when compiling enhanced-for cycle. > > One strange thing I also noticed is static field > `sun.tools.jstat.Parser#reservedWords`, which filled in `Parser` constructor. > Reworked to initialize it once. Marked as reviewed by cjplummer (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5673
Re: RFR: 8274395: Use enhanced-for instead of plain 'for' in jdk.internal.jvmstat
On Mon, 20 Sep 2021 08:15:13 GMT, Andrey Turbanov wrote: > There are few places in code where manual `for` loop is used with Iterator to > iterate over Collection or Array. > Instead of manual `for` cycles it's preferred to use enhanced-for cycle > instead: it's less verbose, makes code easier to read and it's less > error-prone. > It doesn't have any performance impact: javac compiler generates similar code > when compiling enhanced-for cycle. > Similar cleanups: > 1. [JDK-8274016](https://bugs.openjdk.java.net/browse/JDK-8274016) > java.desktop > 2. [JDK-8274237](https://bugs.openjdk.java.net/browse/JDK-8274237) java.base > 3. [JDK-8274261](https://bugs.openjdk.java.net/browse/JDK-8274261) jdk.jcmd > 4. [JDK-8274318](https://bugs.openjdk.java.net/browse/JDK-8274318) > java.management You need a copyright update in TypeCode.java. Otherwise the changes look good. - Marked as reviewed by cjplummer (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5580
RFR: 8274395: Use enhanced-for instead of plain 'for' in jdk.internal.jvmstat
There are few places in code where manual `for` loop is used with Iterator to iterate over Collection or Array. Instead of manual `for` cycles it's preferred to use enhanced-for cycle instead: it's less verbose, makes code easier to read and it's less error-prone. It doesn't have any performance impact: javac compiler generates similar code when compiling enhanced-for cycle. - Commit messages: - [PATCH] Use enhanced-for instead of plain for in jdk.internal.jvmstat Changes: https://git.openjdk.java.net/jdk/pull/5580/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5580=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274395 Stats: 49 lines in 7 files changed: 3 ins; 11 del; 35 mod Patch: https://git.openjdk.java.net/jdk/pull/5580.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5580/head:pull/5580 PR: https://git.openjdk.java.net/jdk/pull/5580
Re: RFR: 8266936: Add a finalization JFR event [v14]
On Sat, 25 Sep 2021 14:22:28 GMT, Markus Grönlund wrote: >> Greetings, >> >> Object.finalize() was deprecated in JDK9. There is an ongoing effort to >> replace and mitigate Object.finalize() uses in the JDK libraries; please see >> https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. >> >> We also like to assist users in replacing and mitigating uses in non-JDK >> code. >> >> Hence, this changeset adds a periodic JFR event to help identify which >> classes are overriding Object.finalize(). >> >> Thanks >> Markus > > Markus Grönlund has updated the pull request incrementally with one > additional commit since the last revision: > > symbols-unix I reviewed the change in java.base that looks good. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8274196: Crashes in VM_HeapDumper::work after JDK-8252842 [v4]
On Mon, 27 Sep 2021 13:06:57 GMT, Per Liden wrote:
>> Lin Zang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> remove redundant empty line
>
> src/hotspot/share/services/heapDumper.cpp line 1601:
>
>> 1599: void JNILocalsDumper::do_oop(oop* obj_p) {
>> 1600: // ignore null handles
>> 1601: oop o = NativeAccess::oop_load(obj_p);
>
> The JNI Local roots do not need a load barrier, only JNI Global roots need
> that. The JNI Local roots are processed on safepoint entry as part of the
> "thread head" (via
> `ZStackWatermark::ZStackWatermark::start_processing_impl()` ->
> `Thread::oops_do_no_frames()`), so once you are in
> `VM_HeapDumper::do_thread()` the JNI Local roots have already passed a load
> barrier.
Get it, Thanks!
-
PR: https://git.openjdk.java.net/jdk/pull/5681
Re: RFR: 8274196: Crashes in VM_HeapDumper::work after JDK-8252842 [v5]
> The root cause for crash in ZGC is that the JNIHandles are processed before > object iteration. And ZGC would update the JNIHandles at object iteration > with read barrier. So the crash is cause by accessing the invalid address > which can be dummy info after zgc, and hence crash. > > The lock rank issue can be fixed because the related mutexes are acquired in > safepoint. so the safepoint_check_required could be safepoint_check_always. > > The Epsilon issue is caused by wrong _num_dumper_thread calculated when the > gang==NULL. Lin Zang has updated the pull request incrementally with one additional commit since the last revision: remove load barrier for JNI local roots - Changes: - all: https://git.openjdk.java.net/jdk/pull/5681/files - new: https://git.openjdk.java.net/jdk/pull/5681/files/f6cb2123..db77cdaa Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5681=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5681=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5681.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5681/head:pull/5681 PR: https://git.openjdk.java.net/jdk/pull/5681
Re: RFR: 8274196: Crashes in VM_HeapDumper::work after JDK-8252842 [v4]
On Mon, 27 Sep 2021 12:02:53 GMT, Lin Zang wrote:
>> The root cause for crash in ZGC is that the JNIHandles are processed before
>> object iteration. And ZGC would update the JNIHandles at object iteration
>> with read barrier. So the crash is cause by accessing the invalid address
>> which can be dummy info after zgc, and hence crash.
>>
>> The lock rank issue can be fixed because the related mutexes are acquired in
>> safepoint. so the safepoint_check_required could be safepoint_check_always.
>>
>> The Epsilon issue is caused by wrong _num_dumper_thread calculated when the
>> gang==NULL.
>
> Lin Zang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> remove redundant empty line
src/hotspot/share/services/heapDumper.cpp line 1601:
> 1599: void JNILocalsDumper::do_oop(oop* obj_p) {
> 1600: // ignore null handles
> 1601: oop o = NativeAccess::oop_load(obj_p);
The JNI Local roots do not need a load barrier, only JNI Global roots need
that. The JNI Local roots are processed on safepoint entry as part of the
"thread head" (via `ZStackWatermark::ZStackWatermark::start_processing_impl()`
-> `Thread::oops_do_no_frames()`), so once you are in
`VM_HeapDumper::do_thread()` the JNI Local roots have already passed a load
barrier.
-
PR: https://git.openjdk.java.net/jdk/pull/5681
Re: RFR: 8202056: Expand serial warning to check for bad overloads of serial-related methods and ineffectual fields
On Mon, 27 Sep 2021 01:00:18 GMT, Joe Darcy wrote:
> This is an initial PR for expanded lint warnings done under two bugs:
>
> 8202056: Expand serial warning to check for bad overloads of serial-related
> methods and ineffectual fields
> 8160675: Issue lint warning for non-serializable non-transient instance
> fields in serializable type
>
> to get feedback on the general approach and test strategy before further
> polishing the implementation.
>
> The implementation initially started as an annotation processor I wrote
> several years ago. The refined version being incorporated into Attr has been
> refactored, had its checks expanded, and been partially ported to idiomatic
> javac coding style rather than using the javax.lang.model API from annotation
> processing.
>
> Subsequent versions of this PR are expected to move the implementation closer
> to idiomatic javac, in particular to use javac flags rather than
> javax.lang.model.Modifier's. Additional resources keys will be defined for
> the serialization-related fields and methods not having the expected
> modifiers, types, etc. The resource keys for the existing checks related to
> serialVersionUID and reused.
>
> Please also review the corresponding CSRs:
>
> https://bugs.openjdk.java.net/browse/JDK-8274335
> https://bugs.openjdk.java.net/browse/JDK-8274336
>
> Informative serialization-related warning messages must take into account
> whether a class, interface, annotation, record, and enum is being analyzed.
> Enum classes and record classes have special handling in serialization. This
> implementation under review has been augmented with checks for interface
> types recommended by Chris Hegarty in an attachment on 8202056.
>
> The JDK build has the Xlint:serial check enabled. The build did not pass with
> the augmented checks. For most modules, this PR contains the library changes
> necessary for the build to pass. I will start separate PRs in those library
> areas to get the needed SuppressWarning("serial") or other changes in place.
> For one module, I temporarily disabled the Xlint:serial check.
>
> In terms of performance, I have not done benchmarks of the JDK build with and
> without these changes, but informally the build seems to take about as long
> as before.
Build change looks ok.
-
Marked as reviewed by erikj (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5709
Re: RFR: 8274196: Crashes in VM_HeapDumper::work after JDK-8252842 [v4]
> The root cause for crash in ZGC is that the JNIHandles are processed before > object iteration. And ZGC would update the JNIHandles at object iteration > with read barrier. So the crash is cause by accessing the invalid address > which can be dummy info after zgc, and hence crash. > > The lock rank issue can be fixed because the related mutexes are acquired in > safepoint. so the safepoint_check_required could be safepoint_check_always. > > The Epsilon issue is caused by wrong _num_dumper_thread calculated when the > gang==NULL. Lin Zang has updated the pull request incrementally with one additional commit since the last revision: remove redundant empty line - Changes: - all: https://git.openjdk.java.net/jdk/pull/5681/files - new: https://git.openjdk.java.net/jdk/pull/5681/files/c1658e19..f6cb2123 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5681=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5681=02-03 Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5681.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5681/head:pull/5681 PR: https://git.openjdk.java.net/jdk/pull/5681
Re: RFR: 8274196: Crashes in VM_HeapDumper::work after JDK-8252842 [v3]
> The root cause for crash in ZGC is that the JNIHandles are processed before > object iteration. And ZGC would update the JNIHandles at object iteration > with read barrier. So the crash is cause by accessing the invalid address > which can be dummy info after zgc, and hence crash. > > The lock rank issue can be fixed because the related mutexes are acquired in > safepoint. so the safepoint_check_required could be safepoint_check_always. > > The Epsilon issue is caused by wrong _num_dumper_thread calculated when the > gang==NULL. Lin Zang has updated the pull request incrementally with one additional commit since the last revision: Add missing read barrier - Changes: - all: https://git.openjdk.java.net/jdk/pull/5681/files - new: https://git.openjdk.java.net/jdk/pull/5681/files/91f1c349..c1658e19 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5681=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5681=01-02 Stats: 34 lines in 1 file changed: 16 ins; 16 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5681.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5681/head:pull/5681 PR: https://git.openjdk.java.net/jdk/pull/5681
Re: RFR: 8274196: Crashes in VM_HeapDumper::work after JDK-8252842 [v2]
On Mon, 27 Sep 2021 09:39:59 GMT, Per Liden wrote:
> > The root cause for crash in ZGC is that the JNIHandles are processed before
> > object iteration. And ZGC would update the JNIHandles at object iteration
> > with read barrier. So the crash is cause by accessing the invalid address
> > which can be dummy info after zgc, and hence crash.
>
> The fix here should not be to change the order of stuff, so that heap
> iteration happens first, that will just hide the real bug. The real bug is
> that the `JNIGlobalsDumper::do_oop()` is missing a load barrier. In other
> words, keep the order and just make sure to add a load barrier, like this:
>
> ```
> void JNIGlobalsDumper::do_oop(oop* obj_p) {
> oop o = NativeAccess::oop_load(obj_p);
> ...
> ```
Hi Per @pliden ,
Thanks a lot！
Correct！I am just puzzling why the sequency of root type dump is a must as
there is no such request in spec, and your suggestion definitely help to answer
that, I took the wrong fix and neglect that there is a read barrier missed.
I will make the change.
BRs,
Lin
-
PR: https://git.openjdk.java.net/jdk/pull/5681
Re: RFR: 8274196: Crashes in VM_HeapDumper::work after JDK-8252842 [v2]
On Sun, 26 Sep 2021 08:02:33 GMT, Lin Zang wrote:
>> The root cause for crash in ZGC is that the JNIHandles are processed before
>> object iteration. And ZGC would update the JNIHandles at object iteration
>> with read barrier. So the crash is cause by accessing the invalid address
>> which can be dummy info after zgc, and hence crash.
>>
>> The lock rank issue can be fixed because the related mutexes are acquired in
>> safepoint. so the safepoint_check_required could be safepoint_check_always.
>>
>> The Epsilon issue is caused by wrong _num_dumper_thread calculated when the
>> gang==NULL.
>
> Lin Zang has updated the pull request with a new target base due to a merge
> or a rebase. The pull request now contains three commits:
>
> - un-ProblemList BasicJMapTest.java
> - Merge branch 'master' into pd-fix
> - 8274196: Crashes in VM_HeapDumper::work after JDK-8252842
> The root cause for crash in ZGC is that the JNIHandles are processed before
> object iteration. And ZGC would update the JNIHandles at object iteration
> with read barrier. So the crash is cause by accessing the invalid address
> which can be dummy info after zgc, and hence crash.
The fix here should not be to change the order of stuff, so that heap iteration
happens first, that will just hide the real bug. The real bug is that the
`JNIGlobalsDumper::do_oop()` is missing a load barrier. In other words, keep
the order and just make sure to add a load barrier, like this:
void JNIGlobalsDumper::do_oop(oop* obj_p) {
oop o = NativeAccess::oop_load(obj_p);
...
-
Changes requested by pliden (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5681
RFR: 8202056: Expand serial warning to check for bad overloads of serial-related methods and ineffectual fields
This is an initial PR for expanded lint warnings done under two bugs:
8202056: Expand serial warning to check for bad overloads of serial-related
methods and ineffectual fields
8160675: Issue lint warning for non-serializable non-transient instance fields
in serializable type
to get feedback on the general approach and test strategy before further
polishing the implementation.
The implementation initially started as an annotation processor I wrote several
years ago. The refined version being incorporated into Attr has been
refactored, had its checks expanded, and been partially ported to idiomatic
javac coding style rather than using the javax.lang.model API from annotation
processing.
Subsequent versions of this PR are expected to move the implementation closer
to idiomatic javac, in particular to use javac flags rather than
javax.lang.model.Modifier's. Additional resources keys will be defined for the
serialization-related fields and methods not having the expected modifiers,
types, etc. The resource keys for the existing checks related to
serialVersionUID and reused.
Please also review the corresponding CSRs:
https://bugs.openjdk.java.net/browse/JDK-8274335
https://bugs.openjdk.java.net/browse/JDK-8274336
Informative serialization-related warning messages must take into account
whether a class, interface, annotation, record, and enum is being analyzed.
Enum classes and record classes have special handling in serialization. This
implementation under review has been augmented with checks for interface types
recommended by Chris Hegarty in an attachment on 8202056.
The JDK build has the Xlint:serial check enabled. The build did not pass with
the augmented checks. For most modules, this PR contains the library changes
necessary for the build to pass. I will start separate PRs in those library
areas to get the needed SuppressWarning("serial") or other changes in place.
For one module, I temporarily disabled the Xlint:serial check.
In terms of performance, I have not done benchmarks of the JDK build with and
without these changes, but informally the build seems to take about as long as
before.
-
Commit messages:
- Appease jcheck
- Implement checks chegar recommended for interfaces.
- Update comment.
- Add tests for instance field checks.
- Clean build with instance field checks in place.
- Merge branch 'master' into JDK-8202056
- Put Externalizable checks last.
- Add checks for constructor access in Serializable classes.
- Add no-arg ctor check for Externalizable classes.
- Improve Externalization warnings.
- ... and 19 more: https://git.openjdk.java.net/jdk/compare/5756385c...d498ff5f
Changes: https://git.openjdk.java.net/jdk/pull/5709/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5709=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8202056
Stats: 1519 lines in 79 files changed: 1511 ins; 1 del; 7 mod
Patch: https://git.openjdk.java.net/jdk/pull/5709.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5709/head:pull/5709
PR: https://git.openjdk.java.net/jdk/pull/5709
Re: RFR: 8274261: Use enhanced-for instead of plain 'for' in jdk.jcmd
On Fri, 24 Sep 2021 09:01:28 GMT, Andrey Turbanov
wrote:
>> src/jdk.jcmd/share/classes/sun/tools/jstat/OptionFormat.java line 81:
>>
>>> 79:
>>> 80: for (Iterator i = children.iterator();
>>> i.hasNext(); /* empty */) {
>>> 81: OptionFormat o = i.next();
>>
>> Why did not you simplify the lines 80-81 the same way as in line 85?
>
> It can't be simplified: it calls `Iterator.hasNext()` inside cycle body.
> `i.hasNext()` at line 82
Okay. Thank you for explanation.
-
PR: https://git.openjdk.java.net/jdk/pull/5673
