Re: RFR: 8287811: JFR: jfr configure error message should not print stack trace
On Fri, 3 Jun 2022 15:54:47 GMT, Erik Gahlin wrote: > Could I have a review of PR that removes a printStackTrace() for the jfr > tool. > > Testing: jdk/jfr/tool > > Thanks > Erik Marked as reviewed by mgronlun (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9018
Re: RFR: 8280844: Epoch shift synchronization point for Compiler threads is inadequate [v2]
> Greetings, > > [JDK-8233111](https://bugs.openjdk.java.net/browse/JDK-8233111) attempted to > address artefact tagging for Compiler threads, letting threads run > _thread_in_native to avoid the transition. Unfortunately, that attempt proved > inadequate. > > The epoch race is avoided only by performing the transition to _thread_in_vm. > > Testing: jdk_jfr > > Thanks > Markus Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision: delegate assertion - Changes: - all: https://git.openjdk.java.net/jdk/pull/8724/files - new: https://git.openjdk.java.net/jdk/pull/8724/files/9ce10130..b4e59b72 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8724&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8724&range=00-01 Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8724.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8724/head:pull/8724 PR: https://git.openjdk.java.net/jdk/pull/8724
RFR: 8280844: Epoch shift synchronization point for Compiler threads is inadequate
Greetings, [JDK-8233111](https://bugs.openjdk.java.net/browse/JDK-8233111) attempted to address artefact tagging for Compiler threads, letting threads run _thread_in_native to avoid the transition. Unfortunately, that attempt proved inadequate. The epoch race is avoided only by performing the transition to _thread_in_vm. Testing: jdk_jfr Thanks Markus - Commit messages: - 8280844 Changes: https://git.openjdk.java.net/jdk/pull/8724/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8724&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280844 Stats: 102 lines in 6 files changed: 14 ins; 84 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8724.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8724/head:pull/8724 PR: https://git.openjdk.java.net/jdk/pull/8724
Re: RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder) [v8]
On Wed, 12 Jan 2022 15:18:46 GMT, Tyler Steele wrote:
>> Just in time for the holidays I have completed an implementation of the JFR
>> functionality for AIX. As a side note, this is my first submission to
>> OpenJDK 👋
>>
>> ### Implementation notes and alternatives considered
>>
>> After modifying the build system to allow the --enable-jvm-feature-jfr to
>> work on AIX, my task was to implement the interfaces from os_perf.hpp. The
>> os_perf_aix.cpp implementation that existed was, I believe, a copy of the
>> Linux implementation. A review of the code in that file showed that
>> NetworkInterface, CPUPerformanceInterface, and SystemProcessInterface would
>> require modification to work on AIX. Using the Linux implementation as a
>> guide, I initially expected to use files from the aix procfs like
>> /proc//psinfo, and /proc//status in a manner similar to the Linux
>> implementation. However, I ended up using libperfstat for all functionality
>> required by the interfaces.
>>
>> ### Testing
>>
>> Testing for JFR seems to be quite light in the repo both before and after
>> this change. In addition to performing manual testing, I have added some
>> basic sanity checks that ensure events can be created and committed (using
>> jtreg), and performs some basic checks on the results of the interface
>> member functions (using gtest).
>>
>> ### More notes
>>
>> I've sent an email to the JFR group about a TOC overflow warning I
>> encountered while building (for the release server target). I believe the
>> fix is to pass -qpic=large when using the xlc toolchain, but my
>> modifications to flags-cflags.m4 have not been successful in removing this
>> warning.
>
> Tyler Steele has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains five commits:
>
> - Merge branch 'master' into JDK-8203290
> - Clean up & testing
>
>- Run JFR tests in test/jdk/jdk/jfr
>- Fix issues found from above test suite
>- Remove unecessary jtreg & gtest tests
>- Remove ineffective `qpic=large`
>- TODO: jdk/jfr/event/runtime/TestNativeLibrariesEvent.java
> - Merge branch 'JDK-8203290' of github.com:backwaterred/jdk into JDK-8203290
> - 8203290: Implements Java Flight Recorder on AIX
>
> - changes build system to allow jfr feature on aix
> - implements NetworkPerformance, CPUPerformance, and SystemProcess
> interfaces from os_perf.hpp
> - implements jfr sanity tests
> - 8203290: Implements Java Flight Recorder on AIX
>
> - changes build system to allow jfr feature on aix
> - implements NetworkPerformance, CPUPerformance, and SystemProcess
> interfaces from os_perf.hpp
> - implements jfr sanity tests
You could test it like this:
In JfrThreadSampler.cpp, you have the OSThreadSampler::protected_task()
function:
Insert the following to crash the system:
void OSThreadSampler::protected_task(const os::SuspendedThreadTaskContext&
context) {
+int* null_ptr = nullptr;
+*null_ptr = 5;
...
If crash protection is enabled and active, for release builds, you will not get
a system crash. Instead, you will get this output (in debug builds you will get
a crash as expected):
[10.740s][error][jfr] Thread method sampler crashed
[10.786s][error][jfr] Thread method sampler crashed
[10.798s][error][jfr] Thread method sampler crashed
[10.810s][error][jfr] Thread method sampler crashed
[10.822s][error][jfr] Thread method sampler crashed
[10.834s][error][jfr] Thread method sampler crashed
[10.847s][error][jfr] Thread method sampler crashed
[10.858s][error][jfr] Thread method sampler crashed
...
-
PR: https://git.openjdk.java.net/jdk/pull/6885
Re: RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder) [v8]
On Wed, 12 Jan 2022 15:18:46 GMT, Tyler Steele wrote: >> Just in time for the holidays I have completed an implementation of the JFR >> functionality for AIX. As a side note, this is my first submission to >> OpenJDK 👋 >> >> ### Implementation notes and alternatives considered >> >> After modifying the build system to allow the --enable-jvm-feature-jfr to >> work on AIX, my task was to implement the interfaces from os_perf.hpp. The >> os_perf_aix.cpp implementation that existed was, I believe, a copy of the >> Linux implementation. A review of the code in that file showed that >> NetworkInterface, CPUPerformanceInterface, and SystemProcessInterface would >> require modification to work on AIX. Using the Linux implementation as a >> guide, I initially expected to use files from the aix procfs like >> /proc//psinfo, and /proc//status in a manner similar to the Linux >> implementation. However, I ended up using libperfstat for all functionality >> required by the interfaces. >> >> ### Testing >> >> Testing for JFR seems to be quite light in the repo both before and after >> this change. In addition to performing manual testing, I have added some >> basic sanity checks that ensure events can be created and committed (using >> jtreg), and performs some basic checks on the results of the interface >> member functions (using gtest). >> >> ### More notes >> >> I've sent an email to the JFR group about a TOC overflow warning I >> encountered while building (for the release server target). I believe the >> fix is to pass -qpic=large when using the xlc toolchain, but my >> modifications to flags-cflags.m4 have not been successful in removing this >> warning. > > Tyler Steele has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - Merge branch 'master' into JDK-8203290 > - Clean up & testing > >- Run JFR tests in test/jdk/jdk/jfr >- Fix issues found from above test suite >- Remove unecessary jtreg & gtest tests >- Remove ineffective `qpic=large` >- TODO: jdk/jfr/event/runtime/TestNativeLibrariesEvent.java > - Merge branch 'JDK-8203290' of github.com:backwaterred/jdk into JDK-8203290 > - 8203290: Implements Java Flight Recorder on AIX > > - changes build system to allow jfr feature on aix > - implements NetworkPerformance, CPUPerformance, and SystemProcess > interfaces from os_perf.hpp > - implements jfr sanity tests > - 8203290: Implements Java Flight Recorder on AIX > > - changes build system to allow jfr feature on aix > - implements NetworkPerformance, CPUPerformance, and SystemProcess > interfaces from os_perf.hpp > - implements jfr sanity tests Hi Tyler, Good work in getting JFR ported - thank you. Most of the work revolved around implementing the platform-specific os_perf* functionality. I had assumed you would need more work concerning the thread sampler, i.e. JFR method sampling, but it seems that os/posix/signals_posix.cpp can be used as-is (for suspend / resume) and that pd_get_top_frame_for_profiling() already have implementations - that's good in that case. Speaking of the thread sampler, to ensure stability, you also need a working implementation of os::ThreadCrashProtection. I can see there is the one in os_posix.cpp, and maybe that is the one already in use? Or does there need to be a specialization in os_aix.cpp? Please see https://bugs.openjdk.java.net/browse/JDK-8279077 for more details about crash protection. I can't comment on the os_perf_aix.cpp changes from a ppc/aix viewpoint - I think Martin has taken care of that already. I am ok with this from a JFR standpoint. Thanks again Markus - Marked as reviewed by mgronlun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6885
Re: RFR: 8257602: Introduce JFR Event Throttling and new jdk.ObjectAllocationSample event (enabled by default)
On Mon, 7 Dec 2020 22:37:20 GMT, Mukilesh Sethu
wrote:
>> Greetings,
>>
>> please help review this enhancement to let JFR sample object allocations by
>> default.
>>
>> A description is provided in the JIRA issue.
>>
>> Thanks
>> Markus
>
> This is great! It would be awesome to have this capability.
>
> One query on the way you are throttling the events (please correct me if I'm
> wrong here as I am new to this codebase),
>
> If I understood correctly, you are throttling the events at the time of
> committing, specifically part of `should_write` or `should_commit` in
> `jfrEvent.hpp`. If so, how would we be able to add throttling to events which
> might require it early on like `ObjectCountAfterGC` or `ObjectCount` events ?
>
> I think it makes perfect sense to have it part of commit for allocation
> events because most of the time consuming tasks like stack walking or storing
> stack trace in global table is done part of event commit and we will be able
> to throttle it. However, for events like `ObjectCountAfterGC` the time
> consuming task is iterating the heap which is unavoidable if we add
> throttling part of commit. So, I am just curious how can we extend this
> solution to such events ?
Hi, @myBloodTop, thanks for your comment.
For more special events (for example periodic events), it will be possible,
although not yet supported, to use the throttling mechanism directly. For
example:
TRACE_REQUEST_FUNC(ObjectCount) {
if (JfrEventThrottler::accept(JfrObjectCountEvent)) {
VM_GC_SendObjectCountEvent op;
VMThread::execute(&op);
}
}
Evaluating the throttle predicate as part of commit or should_commit() is an
optimization to avoid having to take the clock twice, but for cases such as the
above, if you don't pass a timestamp, a timestamp will be taken for you as part
of the evaluation.
Now, ObjectCount and ObjectCountAfterGC are also special in another respect, in
that they are UNTIMED, meaning the events are timestamped outside of JFR. For
other, non-UNTIMED events, it would be sufficient to only use the
should_commit() tester, since the throttler evaluation is incorporated (post
enable and threshold checks evaluations). For example:
MyEvent event;
...
if (event.should_commit()) { <<-- throttle evaluation
event.set_field(...);
event.commit();
}
Thanks
Markus
-
PR: https://git.openjdk.java.net/jdk/pull/1624
Re: RFR: 8257602: Introduce JFR Event Throttling and new jdk.ObjectAllocationSample event (enabled by default) [v4]
On Tue, 8 Dec 2020 16:31:37 GMT, Erik Gahlin wrote:
>> Markus Grönlund has updated the pull request incrementally with 12
>> additional commits since the last revision:
>>
>> - initialization check
>> - thread locals and detach and reattach
>> - Tighter ThrottleUnit
>> - JFC control elements
>> - TLAB include
>> - ThrottleUnit enum
>> - remote tests
>> - jfc control attributes
>> - Sampling frequency adjustment for large objects
>> - Treat large objects as tlabs for sampling purposes
>> - ... and 2 more:
>> https://git.openjdk.java.net/jdk/compare/6918f0c8...4e986552
>
> src/jdk.jfr/share/classes/jdk/jfr/internal/EventControl.java line 79:
>
>> 77: private final PlatformEventType type;
>> 78: private final String idName;
>> 79:
>
> Why move Enabled to later?
Configuring the throttler implementation in native is a bit involved. With the
existing order, with enabled first, events are enabled before subsequent
conditions are set up. For the throttler, it means as soon as the enabled
setting is flipped, the throttler gets traffic, but its not yet configured to
accept it. It has a default, which is off, meaning it accepts all events until
the subsequent call to configure the throttler which can take some time,
because the setup is non-trivial. It was found that rates are not respected
because of the throttler not having been setup yet when its starts to get
traffic.
> src/jdk.jfr/share/classes/jdk/jfr/internal/Utils.java line 229:
>
>> 227: // Expected input format is "x/y" or "x\y" where x is a
>> non-negative long
>> 228: // and y is a time unit. Split the string at the delimiter.
>> 229: private static String parseThrottleString(String s, boolean value) {
>
> I think we should only support one type of slash "/".
Fixed.
> src/jdk.jfr/share/classes/jdk/jfr/internal/Utils.java line 249:
>
>> 247: }
>> 248:
>> 249: private static TimeUnit timeUnit(String unit) {
>
> This could be done with an enum with a constructor.
Fixed.
> src/jdk.jfr/share/classes/jdk/jfr/internal/settings/ThrottleSetting.java line
> 65:
>
>> 63: @Override
>> 64: public String combine(Set values) {
>> 65: double max = OFF;
>
> Probably better to use a long (nanos) than floating number
Fixed.
> src/jdk.jfr/share/classes/jdk/jfr/internal/settings/ThrottleSetting.java line
> 88:
>
>> 86: @Override
>> 87: public void setValue(String s) {
>> 88: this.value = s;
>
> If parsing fails, I think things should be kept as is. At least that is what
> the SettingControl interface say.s
>
> I looked at other setting control and the implementation seems wrong there as
> well.
Fixed.
> src/jdk.jfr/share/conf/jfr/default.jfc line 618:
>
>> 616:
>> 617:
>> 618: true
>
> I think enabled should have the "memory-profiling" control.
Fixed.
> src/jdk.jfr/share/conf/jfr/profile.jfc line 608:
>
>> 606:
>> 607:
>> 608: > control="memory-profiling-enabled-medium">false
>
> Need to sync this with .
>
> Perhaps a new choice are needed "Object Allocation"
New control elements and attributes introduced.
-
PR: https://git.openjdk.java.net/jdk/pull/1624
Re: RFR: 8257602: Introduce JFR Event Throttling and new jdk.ObjectAllocationSample event (enabled by default) [v4]
> Greetings, > > please help review this enhancement to let JFR sample object allocations by > default. > > A description is provided in the JIRA issue. > > Thanks > Markus Markus Grönlund has updated the pull request incrementally with 12 additional commits since the last revision: - initialization check - thread locals and detach and reattach - Tighter ThrottleUnit - JFC control elements - TLAB include - ThrottleUnit enum - remote tests - jfc control attributes - Sampling frequency adjustment for large objects - Treat large objects as tlabs for sampling purposes - ... and 2 more: https://git.openjdk.java.net/jdk/compare/6918f0c8...4e986552 - Changes: - all: https://git.openjdk.java.net/jdk/pull/1624/files - new: https://git.openjdk.java.net/jdk/pull/1624/files/6918f0c8..4e986552 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1624&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1624&range=02-03 Stats: 530 lines in 13 files changed: 174 ins; 285 del; 71 mod Patch: https://git.openjdk.java.net/jdk/pull/1624.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1624/head:pull/1624 PR: https://git.openjdk.java.net/jdk/pull/1624
Re: RFR: 8257602: Introduce JFR Event Throttling and new jdk.ObjectAllocationSample event (enabled by default) [v3]
> Greetings, > > please help review this enhancement to let JFR sample object allocations by > default. > > A description is provided in the JIRA issue. > > Thanks > Markus Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision: ObjectAllocationSample event definition with weight - Changes: - all: https://git.openjdk.java.net/jdk/pull/1624/files - new: https://git.openjdk.java.net/jdk/pull/1624/files/196d254d..6918f0c8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1624&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1624&range=01-02 Stats: 30 lines in 6 files changed: 3 ins; 11 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/1624.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1624/head:pull/1624 PR: https://git.openjdk.java.net/jdk/pull/1624
Re: RFR: 8257602: Introduce JFR Event Throttling and new jdk.ObjectAllocationSample event (enabled by default) [v2]
> Greetings, > > please help review this enhancement to let JFR sample object allocations by > default. > > A description is provided in the JIRA issue. > > Thanks > Markus Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision: General ObjectAllocationSample event definition - Changes: - all: https://git.openjdk.java.net/jdk/pull/1624/files - new: https://git.openjdk.java.net/jdk/pull/1624/files/dba878aa..196d254d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1624&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1624&range=00-01 Stats: 23 lines in 2 files changed: 0 ins; 18 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/1624.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1624/head:pull/1624 PR: https://git.openjdk.java.net/jdk/pull/1624
RFR: 8257602: Introduce JFR Event Throttling and new jdk.ObjectAllocationSample event (enabled by default)
Greetings, please help review this enhancement to let JFR sample object allocations by default. A description is provided in the JIRA issue. Thanks Markus - Commit messages: - defensive initialization check - Whitespace errors - JFR Event Throttling Changes: https://git.openjdk.java.net/jdk/pull/1624/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1624&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8257602 Stats: 2607 lines in 43 files changed: 2346 ins; 238 del; 23 mod Patch: https://git.openjdk.java.net/jdk/pull/1624.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1624/head:pull/1624 PR: https://git.openjdk.java.net/jdk/pull/1624
RE: RFR: 8023492 jfr.jar gets loaded even though it's not used
Looks good Staffan, thanks for fixing. /Markus -Original Message- From: Staffan Larsen Sent: den 30 september 2013 10:00 To: build-dev@openjdk.java.net build-dev; serviceability-...@openjdk.java.net serviceability-...@openjdk.java.net Subject: RFR: 8023492 jfr.jar gets loaded even though it's not used When running a program that has code in the com.oracle.* package, jfr.jar will be loaded even if JFR is not referenced. This is because jre/lib/meta-index says that jfr.jar contains code for com.oracle.*. While this is true, it is a bit too general. It would be more accurate to say that jfr.jar contains code for com.oracle.jrockit.*. Special cases such as this is handled in make/tools/src/build/tools/buildmetaindex/BuildMetaIndex.java. However, the logic started to look a bit complex with all the special handling, so I re-wrote it a little bit. The net result is that jre/lib/meta-index now says: # jfr.jar oracle/jrockit/ com/oracle/jrockit/ jdk/jfr/ instead of: # jfr.jar oracle/jrockit/ jdk/jfr/ com/oracle/ webrev: http://cr.openjdk.java.net/~sla/8023492/webrev.00/ Thanks, /Staffan
RE: RFR(XS): 8019155 Update makefiles with correct jfr packages
Looks good. /Markus -Original Message- From: Staffan Larsen Sent: den 28 juni 2013 15:16 To: build-dev@openjdk.java.net build-dev; serviceability-...@openjdk.java.net serviceability-...@openjdk.java.net Subject: RFR(XS): 8019155 Update makefiles with correct jfr packages Please review this small fix for the 8 makefiles to include the correct packages in jfr.jar. This is a forward port from 7u40, but the package names have changed. webrev: http://cr.openjdk.java.net/~sla/8019198/webrev.00/ Thanks, /Staffan
RE: RFR (XS): 8019155 Update makefiles with correct jfr packages
Looks good. /Markus -Original Message- From: Staffan Larsen Sent: den 26 juni 2013 09:42 To: build-dev@openjdk.java.net build-dev; serviceability-...@openjdk.java.net serviceability-...@openjdk.java.net Subject: RFR (XS): 8019155 Update makefiles with correct jfr packages Please review this small fix for the 7u40 makefiles to include the correct packages in jfr.jar. webrev: http://cr.openjdk.java.net/~sla/8019155/webrev.00/ Thanks, /Staffan
RE: RFR: 7133124 Remove redundant packages from JAR command line
Thanks Alan for quick response, much appreciated. Markus > -Original Message- > From: Alan Bateman > Sent: den 26 januari 2012 10:52 > To: Rickard Bäckman > Cc: serviceability-...@openjdk.java.net; build-dev@openjdk.java.net > Subject: Re: RFR: 7133124 Remove redundant packages from JAR command > line > > On 26/01/2012 09:44, Rickard Bäckman wrote: > > Hi, > > > > We have a problem with some versions of jar reporting errors when > > trying to run jar cf com/test com/test/foo > > > > This fix removes the redundant subdirectories from the command. > > > > Webrev is here: > > http://cr.openjdk.java.net/~rbackman/7133124/webrev/ > > > > Thanks > > /Rickard > Looks okay to me. > > -Alan.
