Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-28 Thread Markus Grönlund
On Thu, 28 Apr 2022 14:44:05 GMT, Erik Gahlin  wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp line 168:
> 
>> 166:   assert(!tl->is_excluded(), "invariant");
>> 167:   if (virtual_thread) {
>> 168: // TODO: blob cache for virtual threads
> 
> Fix this now or after integration?

Well spotted. It is an optimization and can happen after integration. I will 
create an enhancement for tracking; thank you.

> src/hotspot/share/jfr/metadata/metadata.xml line 121:
> 
>> 119: thread="true" stackTrace="true">
>> 120: > description="Thread enlisted as a carrier" />
>> 121: > description="Class of the continuation" />
> 
> "Continuation class" = >" Continuation Class"
> numFrames = frames
> numRefs = references
> "Number of interpreted frames" => "Interpreted Frames"
> "Number of references" => "References"
> "Stack size in bytes" => "Stack Size" contentType"bytes"

Will fix.

> src/hotspot/share/jfr/metadata/metadata.xml line 130:
> 
>> 128: thread="true" stackTrace="true">
>> 129: > description="Thread enlisted as a carrier" />
>> 130: > description="Class of the continuation" />
> 
> contClass => continuationClass
> "Continuation class" => "Continuation Class"
> "Class of the continuation" Remove (not needed)
> "Number of interpreted frames" => "Interpreted Frames"
> numFrames => frames
> "Number of references" => "References"
> numRefs => references
> "Stack size in bytes" => "Stack Size" contentType="bytes"

Will fix.

> src/hotspot/share/jfr/metadata/metadata.xml line 138:
> 
>> 136:   > category="Java Virtual Machine" label="Continuation Freeze Young" 
>> thread="true" stackTrace="false" startTime="false">
>> 137: 
>> 138: 
> 
> "Allocated new" => "Allocated New"

Will fix, thanks.

> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadEndEvent.java line 35:
> 
>> 33: 
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread end")
> 
> "Virtual thread end" => "Virtual Thread End"
> Remove description.

Will fix.

> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadPinnedEvent.java line 
> 35:
> 
>> 33: 
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread pinned")
> 
> "Virtual thread pinned" => "Virtual Thread Pinned"
> Remove description

Will fix.

> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadStartEvent.java line 35:
> 
>> 33: 
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread start")
> 
> "virtual thread start" => "Virtual Thread Start"
> Remove description

Will fix, thanks.

> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadSubmitFailedEvent.java 
> line 35:
> 
>> 33: 
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread submit task failed")
> 
> The label is a bit a long, would "Virtual Thread Submit Failed" work?

It works. Thanks.

-

PR: https://git.openjdk.java.net/jdk/pull/8166


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-28 Thread Markus Grönlund
On Thu, 28 Apr 2022 14:41:04 GMT, Erik Gahlin  wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp line 94:
> 
>> 92: static const size_t global_buffer_size = 512 * K;
>> 93: 
>> 94: static const size_t thread_local_buffer_prealloc_count = 32;
> 
> Why is more memory needed? Moore's law or something specific to virtual 
> threads?

The carrier thread will write the metadata for the mounted virtual threads 
lazily with virtual threads as part of the event write. They write this thread 
locally, and the memory increase accommodates fewer roundtrips to fetch new 
buffers. The previous size was small because it would only hold at most one 
entry.

-

PR: https://git.openjdk.java.net/jdk/pull/8166


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-28 Thread Markus Grönlund
On Thu, 28 Apr 2022 14:37:22 GMT, Erik Gahlin  wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp line 81:
> 
>> 79:   be_writer.write(size);
>> 80:   be_writer.write(time);
>> 81:   be_writer.write(JfrTicks::now().value() - time);
> 
> Why is this changed?

It was a small optimization attempted before the current system of writing 
virtual thread checkpoints in bulk. Will restore it, thank you.

-

PR: https://git.openjdk.java.net/jdk/pull/8166


Re: RFR: 8284444: Sting typo [v3]

2022-04-06 Thread Markus Grönlund
On Wed, 6 Apr 2022 16:47:17 GMT, Daniel Jeliński  wrote:

>> This patch adds missing `r` in `string`s
>
> Daniel Jeliński has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - revert xalan changes
>  - revert icu changes

JFR changes look fine, thank you.

-

Marked as reviewed by mgronlun (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8125


Integrated: 8266936: Add a finalization JFR event

2021-10-18 Thread Markus Grönlund
On Thu, 8 Jul 2021 19:47:26 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

This pull request has now been integrated.

Changeset: 72a976ef
Author:Markus Grönlund 
URL:   
https://git.openjdk.java.net/jdk/commit/72a976ef05fc2c62657920a560a0abc60b27c852
Stats: 1917 lines in 36 files changed: 1375 ins; 409 del; 133 mod

8266936: Add a finalization JFR event

Reviewed-by: coleenp, mchung, egahlin

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v20]

2021-10-18 Thread Markus Grönlund
> 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:

  no constexpr for constant values

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/85a46263..b3268c90

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=19
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=18-19

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v12]

2021-10-18 Thread Markus Grönlund
On Fri, 24 Sep 2021 22:31:18 GMT, Mandy Chung  wrote:

>> Markus Grönlund has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   no precompiled headers and mtServiceability nmt classification
>
> Hi Markus,
> 
> It's a little surprised to see Finalizer.c to depend JMM interface which is 
> used by `java.management` and `jdk.management` modules only.   It's more 
> appropriate for it to be a JVM_* entry point for Finalizer to report 
> completion of the finalization instead.I understand that you want to make 
> FinalizerService to be a conditional feature which is a good idea.  Such JVM 
> entry can be made as a nop if not INCLUDE_SERVICES.

Thank you for staying around with this protracted PR. Thanks @mlchung , 
@coleenp and @egahlin for your reviews!

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v10]

2021-10-18 Thread Markus Grönlund
On Tue, 31 Aug 2021 08:45:54 GMT, Erik Gahlin  wrote:

>> Markus Grönlund has updated the pull request incrementally with three 
>> additional commits since the last revision:
>> 
>>  - localize
>>  - cleanup
>>  - FinalizerStatistics
>
> src/java.base/share/classes/java/lang/ref/Finalizer.java line 71:
> 
>> 69: }
>> 70: 
>> 71: 
> 
> extraneous whitespace?

I think this version is outdated, and the extra whitespace was removed in later 
versions.

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v10]

2021-10-18 Thread Markus Grönlund
On Tue, 31 Aug 2021 08:42:58 GMT, Erik Gahlin  wrote:

>> Markus Grönlund has updated the pull request incrementally with three 
>> additional commits since the last revision:
>> 
>>  - localize
>>  - cleanup
>>  - FinalizerStatistics
>
> test/jdk/jdk/jfr/event/runtime/TestFinalizerStatisticsEvent.java line 98:
> 
>> 96:   case TEST_CLASS_NAME: {
>> 97:   
>> Asserts.assertTrue(event.getString("codeSource").startsWith("file://"));
>> 98:   foundTestClassName = true;
> 
> Could we (sanity) check "objects" and "totalFinalzersRun" fields as well?

It's risky to do because of the non-deterministic nature of when the Finalizer 
thread runs (if at all). The best I could think of is to check if either field 
is 0 or more, but that becomes so weak it's not much of a sanity check.

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v19]

2021-10-18 Thread Markus Grönlund
> 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:

  relax memory ordering constraint

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/d10eb309..85a46263

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=18
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=17-18

  Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v18]

2021-10-16 Thread Markus Grönlund
> 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:

  ThreadBlockInVM instead of ThreadToNativeFromVM for sampling exclusion

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/9b418149..d10eb309

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=17
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=16-17

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v16]

2021-10-15 Thread Markus Grönlund
On Thu, 14 Oct 2021 21:43:27 GMT, Coleen Phillimore  wrote:

>> Markus Grönlund 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.
>
> src/hotspot/share/jfr/support/jfrSymbolTable.cpp line 75:
> 
>> 73: 
>> 74: JfrSymbolTable::JfrSymbolTable() :
>> 75:   _symbol_table(new SymbolTable(this)),
> 
> I'm confused about which symbol table this is.  Is this the same SymbolTable 
> as classfile/symbolTable.cpp? that instance is assumed to be a singleton.  Is 
> this a different SymbolTable and can it have a different name (thought it was 
> JfrSymbolTable).

Renamed.

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v16]

2021-10-15 Thread Markus Grönlund
On Thu, 14 Oct 2021 22:36:30 GMT, Markus Grönlund  wrote:

>> src/hotspot/share/jfr/support/jfrSymbolTable.hpp line 68:
>> 
>>> 66:   template  class, 
>>> typename, size_t>
>>> 67:   friend class HashTableHost;
>>> 68:   typedef HashTableHost>> JfrSymbolTable> SymbolTable;
>> 
>> Oh here it is.  Since it's an enclosed type, can you rename it something 
>> simpler like Symbols and the other Strings?
>
> Maybe :)

Renamed.

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v17]

2021-10-15 Thread Markus Grönlund
> 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 two additional 
commits since the last revision:

 - renames
 - spelling

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/96a9d6bf..9b418149

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=16
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=15-16

  Stats: 80 lines in 6 files changed: 0 ins; 0 del; 80 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v16]

2021-10-14 Thread Markus Grönlund
On Thu, 14 Oct 2021 21:46:36 GMT, Coleen Phillimore  wrote:

>> Markus Grönlund 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.
>
> src/hotspot/share/jfr/support/jfrSymbolTable.hpp line 68:
> 
>> 66:   template  class, 
>> typename, size_t>
>> 67:   friend class HashTableHost;
>> 68:   typedef HashTableHost> JfrSymbolTable> SymbolTable;
> 
> Oh here it is.  Since it's an enclosed type, can you rename it something 
> simpler like Symbols and the other Strings?

Maybe :)

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v10]

2021-10-14 Thread Markus Grönlund
On Thu, 14 Oct 2021 22:02:43 GMT, Coleen Phillimore  wrote:

>> Ok, I see - grow the table.
>> 
>> I'm not sure if you need to ask ik->is_loader_alive() or not, but I think 
>> you do.  The InstanceKlass is removed from your table during class unloading 
>> but before that during concurrent class unloading, the class might not be 
>> alive while you look at it and concurrent class unloading hasn't gotten 
>> around to removing it yet.  Since you save classes regardless of CLD, you 
>> have to check if it's alive.  See classLoaderDataGraph.cpp 
>> ClassLoaderDataGraphIterator for example.  The CLDG_lock only keeps the 
>> graph from not getting modified, but the classes in it might be dead.
>
> That said, I don't see where you return an InstanceKlass in the table, which 
> would need this check.  Not in class_unloading_do because this follows the 
> _unloading list.

So there is already support for concurrent class unloading today in JFR, and 
the protocol is built around the CLDG_lock. If JFR holds it, concurrent class 
unloading cannot run. If GC holds it, JFR cannot inspect classes. That's why 
the table inspection is guarded via the CLDG_lock, for mutual exclusion to 
avoid this problem. I.e. if concurrent class unloading is in progress, JFR will 
not inspect the table.

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v16]

2021-10-13 Thread Markus Grönlund
> 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 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 two new commits 
since the last revision:

 - cleanup
 - rebased and adjusted for new lock rankings

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/5359a793..96a9d6bf

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=15
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=14-15

  Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v15]

2021-10-13 Thread Markus Grönlund
> 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 with a new target base due to a 
merge or a rebase. The pull request now contains 44 commits:

 - Merge branch '8266936-alt' of github.com:mgronlun/jdk into 8266936-alt
 - symbols-unix
 - jvm.h and JVM_Entry
 - no precompiled headers and mtServiceability nmt classification
 - remove rehashing and rely on default grow_hint for table resize
 - mtStatistics
 - localize
 - cleanup
 - FinalizerStatistics
 - Model as finalizerService
 - ... and 34 more: https://git.openjdk.java.net/jdk/compare/124f8237...5359a793

-

Changes: https://git.openjdk.java.net/jdk/pull/4731/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4731=14
  Stats: 1893 lines in 36 files changed: 1375 ins; 409 del; 109 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v14]

2021-09-25 Thread Markus Grönlund
> 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

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/73091f7c..4eaad60d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=13
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=12-13

  Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v12]

2021-09-25 Thread Markus Grönlund
On Fri, 24 Sep 2021 22:31:18 GMT, Mandy Chung  wrote:

> 
> 
> Hi Markus,
> 
> It's a little surprised to see Finalizer.c to depend JMM interface which is 
> used by `java.management` and `jdk.management` modules only. It's more 
> appropriate for it to be a JVM_* entry point for Finalizer to report 
> completion of the finalization instead. I understand that you want to make 
> FinalizerService to be a conditional feature which is a good idea. Such JVM 
> entry can be made as a nop if not INCLUDE_SERVICES.

Thanks Mandy,
 
I choose jmm.h over jvm.h because I wanted the FinalizerService to be 
conditional, like some other services/management implementations in the VM. 
There is conditional compilation support for the 'management' component of the 
JVM, i.e. INCLUDE_MANAGMENT. The jvm.h function JVM_GetManagement(JMM_VERSION) 
returns null if this subsystem is not included in the JVM, making it easier to 
support this as a conditional feature. In addition, I had hoped to see if I 
could also expose some of this finalizer statistics information via JMX. I did 
not get around to that, but a few accessors can now easily be added to jmm.h to 
get to this information and build on it further, should that be useful. There 
is also the benefit of not having to perform a VM call every time, which is 
probably only a small upshot because the code will most often be included. 

These reasons do not seem strong enough to motivate a move outside of the 
original classification, so I am ok to put it into jvm.h instead of jmm.h and 
create a JVM_Entry with a conditionalized body.

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v13]

2021-09-25 Thread Markus Grönlund
> 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:

  jvm.h and JVM_Entry

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/5759c605..73091f7c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=12
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=11-12

  Stats: 52 lines in 6 files changed: 20 ins; 29 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v11]

2021-09-24 Thread Markus Grönlund
On Thu, 23 Sep 2021 22:35:26 GMT, Coleen Phillimore  wrote:

>> Markus Grönlund has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - remove rehashing and rely on default grow_hint for table resize
>>  - mtStatistics
>
> src/hotspot/share/jfr/periodic/jfrFinalizerStatisticsEvent.cpp line 26:
> 
>> 24: 
>> 25: #include "precompiled.hpp"
>> 26: #if INCLUDE_MANAGEMENT
> 
> With precompiled headers turned off, this gets a compilation error:
> error: "INCLUDE_MANAGEMENT" is not
>  defined, evaluates to 0 [-Werror=undef]

Thanks, I missed submitting a few no-precompiled header builds. Fixed now.

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v12]

2021-09-24 Thread Markus Grönlund
> 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:

  no precompiled headers and mtServiceability nmt classification

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/62592daf..5759c605

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=11
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=10-11

  Stats: 9 lines in 4 files changed: 4 ins; 2 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8272515: JFR: Names should only be valid Java identifiers

2021-09-20 Thread Markus Grönlund
On Wed, 8 Sep 2021 13:00:17 GMT, Erik Gahlin  wrote:

> Hi,
> 
> Could I have a review of change that prevents invalid Java identifiers or 
> type names in JFR events. For rationale and compatibility issues see the CSR 
> request. The only change to java.base is making 
> jdk.modules.internal.Checks::isJavaIdentifier(String) public, so it can be 
> reused by the jdk.jfr module.
> 
> Testing: /jdk/jdk/jfr + tier1 + tier2
> 
> Thanks
> Erik

Looks good.

-

Marked as reviewed by mgronlun (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5415


Re: RFR: 8266936: Add a finalization JFR event [v11]

2021-09-17 Thread Markus Grönlund
On Fri, 17 Sep 2021 12:02:32 GMT, Coleen Phillimore  wrote:

>> Markus Grönlund has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - remove rehashing and rely on default grow_hint for table resize
>>  - mtStatistics
>
> src/hotspot/share/runtime/mutexLocker.cpp line 323:
> 
>> 321: 
>> 322: #if INCLUDE_JFR
>> 323:   def(JfrMsg_lock  , PaddedMonitor, leaf-1,  true,  
>> _safepoint_check_always); // -1 because the ConcurrentHashTable resize lock 
>> is leaf
> 
> The ConcurrentHashTable_lock is a lock that doesn't check for safepoints, so 
> if you take this lock out while checking for safepoints, it should assert 
> when called from a JavaThread, which makes me think it's not called from a 
> JavaThread and should be _safepoint_check_never.
>   _resize_lock =
> new Mutex(Mutex::nosafepoint-2, "ConcurrentHashTableResize_lock",
>   Mutex::_safepoint_check_never);
> In my change, this is the ranking for ConcurrentHashTableResize_lock, so this 
> should be nosafepoint-3 if  you check in after I do.

Thanks, the JfrMsg_lock is acquired normally with safepoint checks when the JFR 
system needs to issue synchronous, blocking operations. Asynchronous 
operations, like message notifications, only attempt a try_lock() to avoid 
blocking in the case the lock is contended. The ConcurrentHashTable_lock is 
held during table iteration to prevent resizes from happening. Fortunately, the 
callback processing will only post asynchronous messages (like "Buffer full") 
and therefore will not reach the safepoint check.

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v11]

2021-09-13 Thread Markus Grönlund
> 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 two additional 
commits since the last revision:

 - remove rehashing and rely on default grow_hint for table resize
 - mtStatistics

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/fd86899f..62592daf

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=10
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=09-10

  Stats: 258 lines in 6 files changed: 25 ins; 197 del; 36 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v10]

2021-09-13 Thread Markus Grönlund
On Wed, 8 Sep 2021 17:37:20 GMT, Coleen Phillimore  wrote:

>> Markus Grönlund has updated the pull request incrementally with three 
>> additional commits since the last revision:
>> 
>>  - localize
>>  - cleanup
>>  - FinalizerStatistics
>
> src/hotspot/share/services/classLoadingService.cpp line 80:
> 
>> 78: 
>> 79: size_t ClassLoadingService::compute_class_size(InstanceKlass* k) {
>> 80:   // lifted from ClassStatistics.do_class(Klass* k)
> 
> Can you remove this line? I think that function is gone now.

Thanks Coleen for taking a look at this.

It is used internally by notify_class_loaded() and notify_class_unloaded(); 
therefore I will change it to have internal linkage.

> src/hotspot/share/services/finalizerService.cpp line 44:
> 
>> 42: _ik(ik),
>> 43: _objects_on_heap(0),
>> 44: _total_finalizers_run(0) {}
> 
> Is this hashtable for every InstanceKlass that defines a finalizer?  How many 
> are there generally?  Why couldn't this be a simple hashtable like 
> ResourceHashtable (soon to be renamed) which you can write in only a few 
> lines of code?

This hashtable holds a FinalizerEntry for every InstanceKlass that provides a 
non-empty finalizer method and have allocated at least one object. How many 
there are in general depends on the application. A ResourceHashtable does not 
have the concurrency property required, as lookups and inserts will happen as 
part of object allocation.

> src/hotspot/share/services/finalizerService.cpp line 114:
> 
>> 112: 
>> 113: static inline void added() {
>> 114:   set_atomic(&_count);
> 
> Why isn't Atomic::inc() good enough here? It's used everywhere else.

Our Atomic implementation does not support CAS of a 64-bit value on 32-bit 
platforms (compiles but does not link).

> src/hotspot/share/services/finalizerService.cpp line 123:
> 
>> 121: static inline uintx hash_function(const InstanceKlass* ik) {
>> 122:   assert(ik != nullptr, "invariant");
>> 123:   return primitive_hash(ik);
> 
> If the hash function is primitive_hash, this hash is good enough to not need 
> rehashing.  The only reason for the rehashing for symbol and string table was 
> that the char* had a dumb hash that was faster but could be hacked, so it 
> needed to become a different hashcode.  This doesn't need rehashing.

Thank you for pointing that out. I had assumed this to be a part of the 
syntactic contract for using the ConcurrentHashTable. My wrong assumption was 
because the SymbolTable (which I used as a model) seemed to pass a 
"rehash_warning" bool to the accessor APIs (get, insert). However, looking more 
closely at the signatures in ConcurrentHashTable, that bool is a "grow_hint", 
not "rehash" specifically. Thanks again; I will remove the rehash support code.

> src/hotspot/share/services/finalizerService.cpp line 485:
> 
>> 483: void FinalizerService::purge_unloaded() {
>> 484:   assert_locked_or_safepoint(ClassLoaderDataGraph_lock);
>> 485:   ClassLoaderDataGraph::classes_unloading_do(_unloading);
> 
> Since you remove entries on unloading, I don't see any reason to have any 
> concurrent cleanup.
> You do however need in the lookup code, some code that doesn't find the 
> InstanceKlass if !ik->is_loader_alive()

"Since you remove entries on unloading, I don't see any reason to have any 
concurrent cleanup."
Thank you, that is true. The only concurrent work required will be to grow the 
table.

"You do however need in the lookup code, some code that doesn't find the 
InstanceKlass if !ik->is_loader_alive()"

A precondition is that the code doing the lookup hold the 
ClassLoaderDataGraph_lock or is at a safepoint. Is that still the case? Would 
not purge_unloaded() take out the InstanceKlass from the table, as part of 
unloading, before !ik->is_loader_alive() is published to the outside world?

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v9]

2021-08-27 Thread Markus Grönlund
On Tue, 24 Aug 2021 12:58:29 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:
> 
>   Model as finalizerService

Sorry for the wide distribution, but this became necessary as the PR touches 
many component areas, if only some with minor parts. Below is a high-level 
description of this suggested PR.

// Design
Per class statistics about finalizers is implemented by 
services/finalizerServices. The concept of a "service" is modelled after other 
management components, such as ClassLoadingService and RuntimeService.

Allocation of an object with a class overriding finalize() with a non-empty 
finalize() method, is hooked by the runtime using bytecode 
"_return_register_finalizer", which lets it enter 
InstanceKlass::register_finalizer().
A hook is added to InstanceKlass::register_finalizer() to notify 
FinalizerService which increments the number of "objects_on_heap" for the 
corresponding InstanceKlass.

The dedicated finalizer thread is responsible for running the finalizer methods 
for referent objects whose java.lang.ref.Finalizers have been enqueued for 
finalization by the GC.
It will get a native method to report the completion state of a finalizer back 
to the VM. FinalizerService will then increase the total number of finalizers 
run and decrease the number of outstanding objects on the heap for the 
corresponding InstanceKlass.

Class Unloading support is in place by adding a hook into 
SystemDictionary::do_unloading(). The existing JFR class unloading support is 
extended to also report a FinalizerStatistic event for the class unloading 
before its corresponding FinalizerEntry is purged.

-Xlog:finalize is added for Unified Logging support.

// JFR event output jdk.FinalizerStatistics:
Event:jdk.FinalizerStatistics {
  startTime = 14:22:06.683
  finalizableClass = 
jdk.jfr.event.runtime.TestFinalizerStatisticsEvent$TestClassOverridingFinalize 
(classLoader = app)
  codeSource = 
"file:///D:/utilities/jtreg/runtime_artifacts/work/classes/jdk/jfr/event/runtime/TestFinalizerStatisticsEvent.d/"
  objects = 0
  totalFinalizersRun = 2
}


Event:jdk.FinalizerStatistics {
  startTime = 14:22:07.244
  finalizableClass = jdk.jfr.internal.RepositoryChunk (classLoader = bootstrap)
  codeSource = N/A
  objects = 3
  totalFinalizersRun = 0
}


// Xlog:finalizer output:
[4.517s][info][finalizer] Registered object (0x73fbf594) of class 
jdk.jfr.internal.RepositoryChunk as finalizable
[4.737s][info][finalizer] Registered object (0x7ee4f130) of class 
jdk.jfr.event.runtime.TestFinalizerStatisticsEvent$TestClassOverridingFinalize 
as finalizable
[5.002s][info][finalizer] Finalizer was run for object (0x7ee4f130) of 
class 
jdk.jfr.event.runtime.TestFinalizerStatisticsEvent$TestClassOverridingFinalize
[5.004s][info][finalizer] Registered object (0x56bf003b) of class 
jdk.jfr.internal.RepositoryChunk as finalizable
[5.127s][info][finalizer] Registered object (0x14d51169) of class 
jdk.jfr.event.runtime.TestFinalizerStatisticsEvent$TestClassOverridingFinalize 
as finalizable
[5.325s][info][finalizer] Finalizer was run for object (0x14d51169) of 
class 
jdk.jfr.event.runtime.TestFinalizerStatisticsEvent$TestClassOverridingFinalize
[5.691s][info][finalizer] Registered object (0x2baaa366) of class 
jdk.jfr.internal.RepositoryChunk as finalizable
[5.696s][info][finalizer] Registered object (0x75b10e10) of class 
jdk.jfr.event.runtime.TestFinalizerStatisticsEvent$TestClassOverridingFinalize 
as finalizable
[5.891s][info][finalizer] Finalizer was run for object (0x75b10e10) of 
class 
jdk.jfr.event.runtime.TestFinalizerStatisticsEvent$TestClassOverridingFinalize
[6.121s][info][finalizer] Registered object (0x3c036ecb) of class 
jdk.jfr.internal.ChunksChannel as finalizable
[6.342s][info][finalizer] Finalizer was run for object (0x3c036ecb) of 
class jdk.jfr.internal.ChunksChannel

// Misc
JfrSymbolTable - an existing JFR internal component repackaged and published to 
let other JFR native components re-use the symbol ids (more specifically for 
the CodeSource in this case).

ClassLoadingService - some parts needed to have better conditional compilation 
and runtime support for running the JVM when excluding JVM feature 'ma

Re: RFR: 8266936: Add a finalization JFR event [v10]

2021-08-27 Thread Markus Grönlund
> 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 three 
additional commits since the last revision:

 - localize
 - cleanup
 - FinalizerStatistics

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/e6b786f1..fd86899f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=09
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=08-09

  Stats: 457 lines in 17 files changed: 190 ins; 222 del; 45 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v9]

2021-08-24 Thread Markus Grönlund
> 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:

  Model as finalizerService

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/5da3bb89..e6b786f1

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=08
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=07-08

  Stats: 1153 lines in 13 files changed: 578 ins; 555 del; 20 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v8]

2021-08-23 Thread Markus Grönlund
> 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:

  enqueued data point

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/7d63693f..5da3bb89

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=07
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=06-07

  Stats: 62 lines in 5 files changed: 46 ins; 8 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v7]

2021-08-23 Thread Markus Grönlund
> 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:

  lock ranking

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/d8b829e8..7d63693f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=05-06

  Stats: 26 lines in 3 files changed: 6 ins; 11 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v6]

2021-08-19 Thread Markus Grönlund
> 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 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.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/c8dbca54..d8b829e8

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=04-05

  Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v5]

2021-08-19 Thread Markus Grönlund
> 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 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 20 additional commits 
since the last revision:

 - Merge branches '8266936-alt' and '8266936-alt' of github.com:mgronlun/jdk 
into 8266936-alt
 - remove build directive
 - Code Source attribute for Finalizer event
 - whitespace
 - update
 - update
 - whitespace
 - 8266936-alt: Add a finalization JFR event
 - shortcut calling Jfr::is_recording()
 - 8266936: Add a finalization JFR event
 - ... and 10 more: https://git.openjdk.java.net/jdk/compare/663b1a74...c8dbca54

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/44988036..c8dbca54

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=03-04

  Stats: 65693 lines in 1278 files changed: 54237 ins; 5619 del; 5837 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v4]

2021-07-27 Thread Markus Grönlund
> 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:

  remove build directive

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/58fc3f0a..44988036

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=02-03

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v3]

2021-07-27 Thread Markus Grönlund
> 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 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.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/d4d154e8..58fc3f0a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=01-02

  Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v2]

2021-07-27 Thread Markus Grönlund
> 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 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 16 additional commits 
since the last revision:

 - merge
 - whitespace
 - update
 - update
 - whitespace
 - 8266936-alt: Add a finalization JFR event
 - shortcut calling Jfr::is_recording()
 - 8266936: Add a finalization JFR event
 - Code Source attribute for Finalizer event
 - whitespace
 - ... and 6 more: https://git.openjdk.java.net/jdk/compare/021c6513...d4d154e8

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/9a966f9f..d4d154e8

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=00-01

  Stats: 10639 lines in 412 files changed: 6366 ins; 2700 del; 1573 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event

2021-07-10 Thread Markus Grönlund
On Fri, 9 Jul 2021 18:21:25 GMT, Brent Christian  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 would 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
>
> src/hotspot/share/jfr/metadata/metadata.xml line 1084:
> 
>> 1082:   
>> 1083: 
>> 1084:   > label="Finalizer" thread="false" startTime="false" period="endChunk">
> 
> Would it make sense for this event to be under the "Java Virtual Machine, GC" 
> category?

My argument for placing the event in the Java Application category:

The finalization concept is, or maybe more correctly has been, reified in the 
Java language. The Java programmer decides to enlist the exposed functionality, 
similar to Java language Exceptions and Errors. JFR already represents events 
for these latter concepts in the Java Application category because programmers 
directly interact with and, in contrast to many events in the JVM category, 
have more control over them.

-

PR: https://git.openjdk.java.net/jdk/pull/4731


RFR: 8266936: Add a finalization JFR event

2021-07-09 Thread Markus Grönlund
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 would 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

-

Commit messages:
 - whitespace
 - update
 - update
 - whitespace
 - 8266936-alt: Add a finalization JFR event
 - shortcut calling Jfr::is_recording()
 - 8266936: Add a finalization JFR event

Changes: https://git.openjdk.java.net/jdk/pull/4731/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4731=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8266936
  Stats: 249 lines in 9 files changed: 248 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


FW: RFR(L): 8056049: getProcessCpuLoad() stops working in one process when a different process exits

2014-10-24 Thread Markus Grönlund
Also sending this to core-libs.

 

Thanks in advance

Markus

 

From: Markus Grönlund 
Sent: den 22 oktober 2014 11:44
To: serviceability-...@openjdk.java.net; jmx-...@openjdk.java.net
Subject: RFR(L): 8056049: getProcessCpuLoad() stops working in one process when 
a different process exits

 

Greetings,

 

Kindly asking for reviews for the following changeset.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8056049 

Webrev: http://cr.openjdk.java.net/~mgronlun/8056049/webrev01/ 

 

Description:

 

The issue is  Windows specific. And the problem relates to using the 
Performance Data Helper API (PDH), more specifically how to use the Process 
PDH object in PDH queries:

 

// code comment extract

 

/*

* Working against the Process object and it's related counters is inherently 
problematic

* when using the PDH API:

*

* For PDH, a process is not primarily identified by it's process id,

* but with a sequential number, for example \Process(java#0), \Process(java#1), 


* The really bad part is that this list is reset as soon as one process exits:

* If \Process(java#1) exits, \Process(java#3) now becomes \Process(java#2) etc.

*

* The PDH query api requires a process identifier to be submitted when 
registering

* a query, but as soon as the list resets, the query is invalidated (since the 
name

* changed).

*

* Solution:

* The #number identifier for a Process query can only decrease after process 
creation.

*

* Therefore we create an array of counter queries for all process object 
instances

* up to and including ourselves:

*

* Ex. we come in as third process instance (java#2), we then create and register

* queries for the following Process object instances:

* java#0, java#1, java#2

*

* currentQueryIndexForProcess() keeps track of the current correct query

* (in order to keep this index valid when the list resets from underneath,

* ensure to call getCurrentQueryIndexForProcess() before every query involving

* Process object instance data).

*/

 

I have already fixed this in the VM as of 
https://bugs.openjdk.java.net/browse/JDK-8019921 

 

In the process of fixing this issue now in the JDK, I realized that the 
previous implementation of using PDH in the JDK was a bit convoluted - 
especially if you would like to reuse functionality / add new counters.

 

Therefore this change also includes an overall rewrite of the how the JDK will 
interface with the PDH library, a rewrite of which (hopefully) improves both 
readability and extensibility.

 

I can do a code walkthrough live if anyone is interested to know the exact 
details of this change.

 

Testing completed : Testset SVC (includes jdk_instrument, jdk_management, 
jdk_jmx, jdk_jdi)

 

Thanks in advance

Markus

 


RE: RFR: 8039256 Add sun/jvmstat/monitor/MonitoredVm/CR6672135.java to ProblemList.txt

2014-04-04 Thread Markus Grönlund
Looks good.

/Markus

-Original Message-
From: Staffan Larsen 
Sent: den 4 april 2014 13:33
To: serviceability-...@openjdk.java.net serviceability-...@openjdk.java.net; 
core-libs-dev Libs
Subject: RFR: 8039256 Add sun/jvmstat/monitor/MonitoredVm/CR6672135.java to 
ProblemList.txt

Please review the change below to add a test to ProblemList.txt. For details, 
see https://bugs.openjdk.java.net/browse/JDK-8033104

Thanks,
/Staffan



diff --git a/test/ProblemList.txt b/test/ProblemList.txt
--- a/test/ProblemList.txt
+++ b/test/ProblemList.txt
@@ -273,4 +273,7 @@
 # 8031482
 sun/tools/jcmd/TestJcmdSanity.java windows-all

+# 8033104
+sun/jvmstat/monitor/MonitoredVm/CR6672135.java generic-all
+
 


RE: RFR: Remove tests from ProblemList.txt

2013-08-21 Thread Markus Grönlund
Thanks for taking a look at these old bugs Staffan.

Looks good (not a Reviewer).

/Markus



-Original Message-
From: Staffan Larsen 
Sent: den 21 augusti 2013 16:21
To: core-libs-dev@openjdk.java.net; serviceability-...@openjdk.java.net 
serviceability-...@openjdk.java.net
Subject: RFR: Remove tests from ProblemList.txt

I've had a look at these two old bugs:
  http://bugs.sun.com/view_bug.do?bug_id=7020857
  http://bugs.sun.com/view_bug.do?bug_id=6909804

They are both listed in ProblemList.txt and reported as intermittent failures 
in 2011 and 2009 respectively.

I've been searching our test history for instances of these failures, but 
cannot find anything for the last year (didn't look further back). I can see 
that the tests have been running and I can see other failures in the tests 
(environment issues mostly) but not the failures reported in the bugs.

I have been doing reruns on linux x86-64 and window x86-64. 1000 iterations did 
not reproduce the problems. 

I would thus like to close the bugs and remove the tests from ProblemList.txt. 
Below is the proposed diff.

Thanks,
/Staffan



diff --git a/test/ProblemList.txt b/test/ProblemList.txt
--- a/test/ProblemList.txt
+++ b/test/ProblemList.txt
@@ -336,12 +336,6 @@
 # Filed 6653793
 com/sun/jdi/RedefineCrossEvent.java generic-all

-# Filed 6987312
-com/sun/jdi/DoubleAgentTest.javageneric-all
-
-# Filed 7020857
-com/sun/jdi/FieldWatchpoints.java   generic-all
-
 # Filed 6402201
 com/sun/jdi/ProcessAttachTest.shgeneric-all