Integrated: 8287811: JFR: jfr configure error message should not print stack trace

[Erik Gahlin]

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

This pull request has now been integrated.

Changeset: 41d5809c
Author:Erik Gahlin 
URL:   
https://git.openjdk.java.net/jdk/commit/41d5809caff0a219c2153fe88d0c61c4b5eba62c
Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod

8287811: JFR: jfr configure error message should not print stack trace

Reviewed-by: mgronlun

-

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

RFR: 8287811: JFR: jfr configure error message should not print stack trace

[Erik Gahlin]

Could I have a review of PR that removes a printStackTrace() for the jfr tool. 

Testing: jdk/jfr/tool

Thanks
Erik

-

Commit messages:
 - Initial

Changes: https://git.openjdk.java.net/jdk/pull/9018/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=9018&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8287811
  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/9018.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/9018/head:pull/9018

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

Re: RFR: 8280844: Epoch shift synchronization point for Compiler threads is inadequate [v2]

[Erik Gahlin]

On Mon, 16 May 2022 11:37:43 GMT, Markus Grönlund  wrote:

>> 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

Marked as reviewed by egahlin (Reviewer).

-

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

Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]

[Erik Gahlin]

On Wed, 11 May 2022 12:59:49 GMT, Magnus Ihse Bursie  wrote:

>> Adam Sotona has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8244681: Add a warning for possibly lossy conversion in compound 
>> assignments
>>   recommended correction of the warning description
>
> make/modules/jdk.jfr/Java.gmk line 26:
> 
>> 24: #
>> 25: 
>> 26: DISABLED_WARNINGS_java += exports lossy-conversions
> 
> Note that with the fix of JDK-8286392 (and JDK-8286396) the 
> `lossy-conversions` warning should not be disabled for the JFR code. 
> 
> In general, you need to check which of the subtasks of JDK-8286374 that has 
> been fixed, and adjust the makefiles accordingly, before pushing this fix.
> 
> (In the future, it might be easier to push the fix which disables the 
> warnings first, and then file follow-up bugs on aa per-component basis, and 
> remind them to remove the disabling in the makefile. That way there won't be 
> a race between individual fixes and a "master" bug like this.)

I agree, but if it doesn't happen, I can follow up with a separate PR where I 
remove the disablement.

-

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

Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]

[Erik Gahlin]

On Wed, 11 May 2022 07:45:39 GMT, Adam Sotona  wrote:

>> Please review this patch adding new lint option, **lossy-conversions**, to 
>> javac to warn about type casts in compound assignments with possible lossy 
>> conversions.
>> 
>> The new lint warning is shown if the type of the right-hand operand of a 
>> compound assignment is not assignment compatible with the type of the 
>> variable.
>> 
>> The implementation of the warning is based on similar check performed to 
>> emit "possible lossy conversion" compilation error for simple assignments. 
>> 
>> Proposed patch also include complex matrix-style test with positive and 
>> negative test cases of lossy conversions in compound assignments.
>> 
>> Proposed patch also disables this new lint option in all affected JDK 
>> modules and libraries to allow smooth JDK build. Individual cases to address 
>> possibly lossy conversions warnings in JDK are already addressed in a 
>> separate umbrella issue and its sub-tasks.
>> 
>> Thanks for your review,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8244681: Add a warning for possibly lossy conversion in compound assignments
>   recommended correction of the warning description

Lossy conversion issues for jdk.jfr and jdk.management.jfr. have been fixed.

-

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

Re: RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder) [v7]

[Erik Gahlin]

On Fri, 14 Jan 2022 16:18:37 GMT, Tyler Steele  wrote:

> > I'm looking for some clarification on the purpose 
> > TestNativeLibrariesEvent.java. Currently it fails with "Missing 
> > libraries:libjvm.so, libjava.so, libzip.so: expected true, was false". I 
> > find those libs in my build directory, and everything else seems to work. 
> > Can anyone provide an idea why this test is failing (or what the failure 
> > means)?
> 
> The only item keeping me from `/integrating` is the failing test referenced 
> above. Can anyone provide some clarification? [Edit: The test is checking a 
> (JFR) recording for the libs, not the path as I misunderstood earlier].
> 
> Another option is to simply add the test to a ProblemList.txt, but I'd prefer 
> not to unless it's really warranted.

What is the output if you run the test on AIX? 

It should print all NativeLibrary events. If you don't get any, you may want to 
look at how data is gathered for EventNativeLibrary.  If you get events, you 
might want to add one library as a sanity test.

-

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

RFR: 8279825: JFR: JFCModel should use SecuritySupport for predefined .jfc files

[Erik Gahlin]

Hi,

Could I have review of a fix that allows the new parser, used by JFCModel, to 
read known configuration files in JAVA_HOME/lib/jfr without FilePermission, 
similar to the older parser.

Testing: jdk/jdkjfr

Thanks
Erik

-

Commit messages:
 - Merge branch 'openjdk:master' into smstart
 - Remove whitespace
 - Initial

Changes: https://git.openjdk.java.net/jdk/pull/7066/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7066&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8279825
  Stats: 53 lines in 4 files changed: 31 ins; 13 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7066.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7066/head:pull/7066

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

Re: RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder)

[Erik Gahlin]

On Fri, 17 Dec 2021 19:07:54 GMT, Tyler  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.

I don't have access to AIX, but there are 500+ tests in test/jdk/jdk/jfr you 
might want to try out.

-

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

RFR: 8274559: JFR: Typo in 'jfr help configure' text

[Erik Gahlin]

Hi,

Could I have review of a typo. 

Testing: jdk/jdk/jfr

Thanks
Erik

-

Commit messages:
 - Initial

Changes: https://git.openjdk.java.net/jdk/pull/5803/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5803&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8274559
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5803.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5803/head:pull/5803

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

Re: RFR: 8269823: JFR: Eliminate 'is_large' check for native JFR event if the size range is certain [v2]

[Erik Gahlin]

On Sun, 4 Jul 2021 08:32:03 GMT, Denghui Dong  wrote:

> ```
> BenchmarkMode  SamplesScore  Score error  Units
> o.s.MyBenchmark.testEmitthrpt   20  8251457.275 2707.578  ops/s
> ```
> 
> without this patch:

That sounds too much. Could you please show the test, so I could try it out.  
The large check should only happen once, so it is amortised. From that point on 
it should basically be the same.

-

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

Re: RFR: 8269823: JFR: Eliminate 'is_large' check for native JFR event if the size range is certain [v2]

[Erik Gahlin]

On Sat, 3 Jul 2021 00:46:13 GMT, Denghui Dong  wrote:

>> Hi,
>> 
>> Could I have a review of this improvement that eliminates 'is_large' check 
>> if the event size range is certain?
>> 
>> JDK-8246260 introduced event large checks to reduce the recording size.
>> This check could be eliminated at compile/build time when one of the 
>> following conditions is satisfied:
>> 1. if the max size is < 128
>> 2. if the min size is >= 128
>> 
>> The max size and the min size could be computed for the most native events 
>> at the generation phase.
>> 
>> And I think this improvement may also be done for JDK events.
>
> Denghui Dong has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   refactor

This complicates the implementation with what I would assume negligible impact 
on performance. Do you have measurements that proves that throughput is 
improved significantly with this change?

-

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

Re: RFR: 8257602: Introduce JFR Event Throttling and new jdk.ObjectAllocationSample event (enabled by default) [v4]

[Erik Gahlin]

On Wed, 9 Dec 2020 20:58:48 GMT, Markus Grönlund  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
>
> 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?

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 "/".

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.

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

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.

src/jdk.jfr/share/conf/jfr/default.jfc line 618:

> 616: 
> 617: 
> 618:   true

I think enabled should have the "memory-profiling" control.

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"

-

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

Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v5]

[Erik Gahlin]

On Wed, 23 Sep 2020 18:41:06 GMT, Philippe Marschall 
 wrote:

>> Hello, newbie here
>> 
>> I picked JDK-8138732 to work on because it has a "starter" label and I 
>> believe I understand what to do.
>> 
>> - I tried to update the copyright year to 2020 in every file.
>> - I decided to change `@since` from 9 to 16 since it is a new annotation 
>> name in a new package.
>> - I tried to keep code changes to a minimum, eg not change to imports if 
>> fully qualified class names are used instead of
>>   imports. In some cases I did minor reordering of imports to keep them 
>> sorted alphabetically.
>> - All tier1 tests pass.
>> - One jpackage/jlink tier2 test fails but I don't believe it is related.
>> - Some tier3 Swing tests fail but I don't think they are related.
>
> Philippe Marschall has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now
> contains one commit:
>   8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate

Marked as reviewed by egahlin (Reviewer).

-

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

Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]

[Erik Gahlin]

On Sat, 12 Sep 2020 00:19:00 GMT, Vladimir Kozlov  wrote:

>> Philippe Marschall has refreshed the contents of this pull request, and 
>> previous commits have been removed. The
>> incremental views will show differences compared to the previous content of 
>> the PR. The pull request contains one new
>> commit since the last revision:
>>   8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate
>
> Marked as reviewed by kvn (Reviewer).

Have you run the JFR tests in test/jdk/jdk/jfr?

-

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

Re: RFR (16): JFR: Separate metadata per JVM-feature

[Erik Gahlin]

Thanks for the explanation.

I think it would better to keep things as is going forward.

Erik

> On 23 Jul 2020, at 22:29, Roman Kennke  wrote:
> 
> Hi Erik,
> 
> 
>> Hi Roman,
>> 
>> What is the problem you are trying to solve? 
>> 
> 
> It is mostly an exercise in cleanliness. In the effort to upstream
> Shenandoah GC to jdk11u (which is still ongoing), it has been asked to
> make sure that build with Shenandoah excluded (--with-jvm-features=-
> shenandoahgc) is identical to current build without Shenandoah patch.
> 
> The symbols and empty method(s) compiled in by JFR have been one of a
> few places that needed some work. With this patch and a few more, I was
> able to *prove mechanically* that the objects compiled by unpatched
> JDK11u are byte-identical to patched JDK11u with Shenandoah disabled.
> 
> I thought I'd offer this here, maybe it's equally interesting going
> forward. If it's agreed that it is not very interesting then be it - I
> don't have a strong opinion about it.
> 
> Thanks & cheers,
> Roman
> 
>> Having metadata per JVM-features adds complexity with little added
>> benefit from what I can see. 
> 
> 
>> Thanks
>> Erik
>> 
>>> On 23 Jul 2020, at 19:48, Roman Kennke  wrote:
>>> 
>>> This is a fall-out from my Shenandoah upstreaming work in JDK11,
>>> where
>>> I made a similar change in order to separate-out Shenandoah parts
>>> from
>>> JFR build when Shenandoah is disabled.
>>> 
>>> Currently, all JFR metadata is collected in a single metadata.xml
>>> file.
>>> From those, the build machinery generates headers and some other
>>> things
>>> from it. For JVM-feature-specific metadata, this leads to stuff
>>> generated that doesn't exist when the feature is excluded from the
>>> build. For example, when disabling Shenandoah, we still need to
>>> compile
>>> empty methods in jfrPeriodic.cpp to satisfy the generated code. 
>>> 
>>> The fix is to extend the code-generator to accept multiple
>>> metadata-
>>> *.xml files and concatenate the parsing. The version in JDK16
>>> accepts a
>>> --xml $FILENAME argument, and I'm extending this to --xml
>>> $FILENAME1:$FILENAME2:.. etc, i.e. multiple filenames separated by
>>> colon. It allows empty filenames, e.g. metadata.xml::: would be
>>> parsed
>>> as a single file metadata.xml files. That makes the build machinery
>>> much simpler.
>>> 
>>> I also did the relevant metadata-separation for Shenandoah because
>>> that
>>> is what I care about myself. If you'd like other parts treated the
>>> same, just let me know.
>>> 
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8250218
>>> Webrev:
>>> http://cr.openjdk.java.net/~rkennke/JDK-8250218/webrev.00/
>>> 
>>> Testing: build with and without Shenandoah, run some tests, I'd
>>> await
>>> submit-repo results before pushing.
>>> 
>>> Can I please get a review?
>>> 
>>> Thanks!
>>> Roman
>>> 
> 

Re: RFR (16): JFR: Separate metadata per JVM-feature

[Erik Gahlin]

Hi Roman,

What is the problem you are trying to solve? 

Speed up compilation, or exclude metadata from recordings?

Having metadata per JVM-features adds complexity with little added benefit from 
what I can see. 

Thanks
Erik

> On 23 Jul 2020, at 19:48, Roman Kennke  wrote:
> 
> This is a fall-out from my Shenandoah upstreaming work in JDK11, where
> I made a similar change in order to separate-out Shenandoah parts from
> JFR build when Shenandoah is disabled.
> 
> Currently, all JFR metadata is collected in a single metadata.xml file.
> From those, the build machinery generates headers and some other things
> from it. For JVM-feature-specific metadata, this leads to stuff
> generated that doesn't exist when the feature is excluded from the
> build. For example, when disabling Shenandoah, we still need to compile
> empty methods in jfrPeriodic.cpp to satisfy the generated code. 
> 
> The fix is to extend the code-generator to accept multiple metadata-
> *.xml files and concatenate the parsing. The version in JDK16 accepts a
> --xml $FILENAME argument, and I'm extending this to --xml
> $FILENAME1:$FILENAME2:.. etc, i.e. multiple filenames separated by
> colon. It allows empty filenames, e.g. metadata.xml::: would be parsed
> as a single file metadata.xml files. That makes the build machinery
> much simpler.
> 
> I also did the relevant metadata-separation for Shenandoah because that
> is what I care about myself. If you'd like other parts treated the
> same, just let me know.
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8250218
> Webrev:
> http://cr.openjdk.java.net/~rkennke/JDK-8250218/webrev.00/
> 
> Testing: build with and without Shenandoah, run some tests, I'd await
> submit-repo results before pushing.
> 
> Can I please get a review?
> 
> Thanks!
> Roman
> 

Re: RFR: 8246436: JFR: Avoid parsing metadata.xml during startup

[Erik Gahlin]

Thanks for all the reviews and the measurements!

Erik

> On 18 Jun 2020, at 13:56, Claes Redestad  wrote:
> 
> Hi Erik,
> 
> looks good - and a great improvement!
> 
> By my measures this removes about 12% of the total JVM cpu use when
> running a Hello World with -XX:StartFlightRecording - or roughly 35ms
> off the total runtime[1]
> 
> /Claes
> 
> [1]
> perf stat -r 25 java -XX:StartFlightRecording HelloWorld
> 
> Before:
>   2622.928325  task-clock (msec) #5.092 CPUs utilized 
>( +-  1.13% )
> 6,654  context-switches  #0.003 M/sec 
>   ( +-  2.36% )
>30  cpu-migrations#0.012 K/sec 
>   ( +-  5.10% )
>29,573  page-faults   #0.011 M/sec 
>   ( +-  1.40% )
> 5,952,854,795  cycles#2.270 GHz   
> ( +-  1.10% )
> 6,431,402,332  instructions  #1.08  insn per cycle
>( +-  0.87% )
> 1,285,805,882  branches  #  490.218 M/sec 
>   ( +-  0.87% )
>50,742,706  branch-misses #3.95% of all branches   
>( +-  0.81% )
> 
>   0.515076132 seconds time elapsed  ( +-  1.41% )
> 
> After:
>   2300.679150  task-clock (msec) #4.778 CPUs utilized 
>( +-  1.28% )
> 6,509  context-switches  #0.003 M/sec 
>   ( +-  2.19% )
>29  cpu-migrations#0.012 K/sec 
>   ( +-  5.23% )
>28,050  page-faults   #0.012 M/sec 
>   ( +-  1.78% )
> 5,177,014,775  cycles#2.250 GHz   
> ( +-  1.32% )
> 5,573,218,149  instructions  #1.08  insn per cycle
>( +-  1.07% )
> 1,116,169,965  branches  #  485.148 M/sec 
>   ( +-  1.08% )
>44,986,319  branch-misses #4.03% of all branches   
>( +-  0.85% )
> 
>   0.481467615 seconds time elapsed  ( +-  1.27% )
> 
> 
> On 2020-06-18 01:16, Erik Gahlin wrote:
>> Hi,
>> Could I have review of an enhancement that will avoid parsing the XML 
>> metadata for the JVM events during startup/runtime.
>> Thanks to Magnus for helping out with changes to the make files.
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8246436
>> Webrev:
>> http://cr.openjdk.java.net/~egahlin/8246436/
>> Testing: tier1+tier2 + jdk/jdk/jfr
>> Thanks
>> Erik

RFR: 8246436: JFR: Avoid parsing metadata.xml during startup

[Erik Gahlin]

Hi,

Could I have review of an enhancement that will avoid parsing the XML metadata 
for the JVM events during startup/runtime.

Thanks to Magnus for helping out with changes to the make files. 

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

Webrev:
http://cr.openjdk.java.net/~egahlin/8246436/

Testing: tier1+tier2 + jdk/jdk/jfr

Thanks
Erik

Re: RFR: 8216303: JFR: Simplify generated files

[Erik Gahlin]

Thanks for the reviews.

Erik

> On 28 May 2020, at 22:55, Erik Joelsson  wrote:
> 
> 
> On 2020-05-28 13:51, Erik Gahlin wrote:
>> Hello Erik,
>> 
>>> On 28 May 2020, at 19:47, Erik Joelsson  wrote:
>>> 
>>> Hello Erik,
>>> 
>>> I noticed that you added an import for java.util.HashSet, but it doesn't 
>>> seem to be used.
>> Fixed.
>> 
>>> You also replaced the only use of HashMap with LinkedHashMap, which we like 
>>> as it gives a predictable iteration order, but forgot to remove the import.
>>> 
>>> Do you know if the generated files are stable over several builds?
>> They should be stable. Events are numbered after how they are specified in 
>> /hotspot/share/jfr/metadata/metadata.xml.
> 
> Good thanks! Then I'm happy with this.
> 
> /Erik
> 
>> Erik
>> 
>>> /Erik
>>> 
>>> On 2020-05-28 10:37, Erik Gahlin wrote:
>>>> Hi,
>>>> 
>>>> Could I have a review of a fix that removes legacy in the generated JFR 
>>>> event files, in particular hard coded numbers for type / event IDs. Event 
>>>> IDs are now numbered from zero, which has the benefit that most of them 
>>>> can be represented as a single byte instead of two. This will reduce the 
>>>> size of recordings with a few percent.
>>>> 
>>>> I’m looping in build-dev, since I am modifying 
>>>> make/src/classes/build/tools/jfr/GenerateJfrFiles.java, but there are no 
>>>> changes to how things are built. The generated JFR files are just slightly 
>>>> different.
>>>> 
>>>> Old and new files:
>>>> http://cr.openjdk.java.net/~egahlin/8216303_generated/
>>>> 
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8216303
>>>> 
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~egahlin/8216303/
>>>> 
>>>> Testing:
>>>> - Building with and without jfr the feature enabled
>>>> - Tier 1/2 (Mac, Linux and Windows)
>>>> - JFR tests in test/jdk/jdk/jfr
>>>> 
>>>> Thanks
>>>> Erik
>>>> 
>>>> 

Re: RFR: 8216303: JFR: Simplify generated files

[Erik Gahlin]

Hello Erik, 

> On 28 May 2020, at 19:47, Erik Joelsson  wrote:
> 
> Hello Erik,
> 
> I noticed that you added an import for java.util.HashSet, but it doesn't seem 
> to be used.

Fixed.

> You also replaced the only use of HashMap with LinkedHashMap, which we like 
> as it gives a predictable iteration order, but forgot to remove the import.
> 
> Do you know if the generated files are stable over several builds?

They should be stable. Events are numbered after how they are specified in 
/hotspot/share/jfr/metadata/metadata.xml.

Erik

> 
> /Erik
> 
> On 2020-05-28 10:37, Erik Gahlin wrote:
>> Hi,
>> 
>> Could I have a review of a fix that removes legacy in the generated JFR 
>> event files, in particular hard coded numbers for type / event IDs. Event 
>> IDs are now numbered from zero, which has the benefit that most of them can 
>> be represented as a single byte instead of two. This will reduce the size of 
>> recordings with a few percent.
>> 
>> I’m looping in build-dev, since I am modifying 
>> make/src/classes/build/tools/jfr/GenerateJfrFiles.java, but there are no 
>> changes to how things are built. The generated JFR files are just slightly 
>> different.
>> 
>> Old and new files:
>> http://cr.openjdk.java.net/~egahlin/8216303_generated/
>> 
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8216303
>> 
>> Webrev:
>> http://cr.openjdk.java.net/~egahlin/8216303/
>> 
>> Testing:
>> - Building with and without jfr the feature enabled
>> - Tier 1/2 (Mac, Linux and Windows)
>> - JFR tests in test/jdk/jdk/jfr
>> 
>> Thanks
>> Erik
>> 
>> 

RFR: 8216303: JFR: Simplify generated files

[Erik Gahlin]

Hi,

Could I have a review of a fix that removes legacy in the generated JFR event 
files, in particular hard coded numbers for type / event IDs. Event IDs are now 
numbered from zero, which has the benefit that most of them can be represented 
as a single byte instead of two. This will reduce the size of recordings with a 
few percent.

I’m looping in build-dev, since I am modifying 
make/src/classes/build/tools/jfr/GenerateJfrFiles.java, but there are no 
changes to how things are built. The generated JFR files are just slightly 
different. 

Old and new files:
http://cr.openjdk.java.net/~egahlin/8216303_generated/

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

Webrev:
http://cr.openjdk.java.net/~egahlin/8216303/

Testing: 
- Building with and without jfr the feature enabled
- Tier 1/2 (Mac, Linux and Windows)
- JFR tests in test/jdk/jdk/jfr

Thanks
Erik

Re: RFR: JDK-8239789 Follow-up on JVM feature rewrite

[Erik Gahlin]

jfr should be JDK Flight Recorder.

Erik

On 2020-02-21 16:05, Magnus Ihse Bursie wrote:
The JVM feature rewrite was actually somewhat of a step backwards in 
terms of presenting information for the user what different configure 
arguments do.


I've compensated for this by making it far better! :) Now we get a 
list like this:


  aot enable ahead of time compilation (AOT)
  cds enable class data sharing (CDS)
  compiler1   enable hotspot compiler C1
  compiler2   enable hotspot compiler C2
  dtrace  enable dtrace support
  epsilongc   include the epsilon (no-op) garbage collector
  g1gc    include the G1 garbage collector
  graal   enable Graal (jdk.internal.vm.compiler)
  jfr enable Java Flight Recorder (JFR)
  jni-check   enable -Xcheck:jni support
  jvmci   enable JVM Compiler Interface (JVMCI)
  jvmti   enable Java Virtual Machine Tool Interface 
(JVM TI)

  link-time-opt   enable link time optimization
  management  enable java.lang.management API support
  minimal support building variant 'minimal'
  nmt include native memory tracking (NMT)
  parallelgc  include the parallel garbage collector
  serialgc    include the serial garbage collector
  services    enable diagnostic services and client attaching
  shenandoahgc    include the Shenandoah garbage collector
  static-build    build static library instead of dynamic
  vm-structs  export JVM structures to the Serviceablility 
Agent

  zero    support building variant 'zero'
  zgc include the ZGC garbage collector

Also, I missed using the recommended m4_ prefix on some m4 functions. 
This patch also corrects this.


Bug: https://bugs.openjdk.java.net/browse/JDK-8239789
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8239789-jvm-features-followup/webrev.01


/Magnus

Re: RFR(S) : 8228448 : Jconsole can't connect to itself

[Erik Gahlin]

Looks good.

Erik


Hi all,

 Please review this small patch.

Added "-Djdk.attach.allowAttachSelf=true" to jconsole launcher make file. This 
will allow jconsole to connect to itself.

  


JBS: https://bugs.openjdk.java.net/browse/JDK-8228448

Webrev: http://cr.openjdk.java.net/~vaibhav/8228448/webrev.00/

Testing: tested with jconsole

  


Thanks,

Ram

Re: RFR(XL): 8199712: Flight Recorder

[Erik Gahlin]

On 2018-05-14 18:05, Erik Joelsson wrote:

Oh, I missed the new makefiles last time I looked at this.

in Copy-jdk.jfr.gmk, everything looks like it's indented an extra 4 
steps. I'm assuming this is because it used to be conditional in the 
previous closed file.

Will fix!



GensrcJfr.gmk, line 94, please move )) to the left.


Will fix!

Looking closer at GensrcJfr.gmk, The macro SetupJfrGeneration looks 
like it is only called once. This could be greatly simplified by just 
taking the body of the macro and inlining all the inputs. This of 
course unless you see a need in the future to generate additional 
files using the jfr tool.

Will fix this in a follow up bug:
https://bugs.openjdk.java.net/browse/JDK-8203221

Thanks
Erik



/Erik

On 2018-05-14 07:36, Erik Gahlin wrote:

Here is an updated webrev:

http://cr.openjdk.java.net/~egahlin/8199712.1/ [1]

that incorporates:

- build changes
- new event prefix, i.e. "com.oracle.jdk.CPULoad" becomes "jdk.CPULoad"
- obsolete command line options EnableTracing and UseLockedTracing
- fixed typos in the Javadoc
- simplified #include files

RFEs have been filed for other issues, CSR is approved and tests pass.

Erik and Markus

[1] Parent:

changeset:   50092:0e42d3120e51

user:clanger
date:Sat May 12 10:26:42 2018 +0200
summary: 8202915: [JAXP] Performance enhancements and cleanups in 
com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator




Greetings,

Could I have a review of 8199712: Flight Recorder

As mentioned in the preview [1] the tracing backend has been 
removed. Event metadata has been consolidated into a single XML file 
and event classes are now generated by GenerateJfrFiles.java.


Tests have been run on Linux-x64, Windows-x64 and MaxOSX-x64.

For details about the feature, see the JEP:
https://bugs.openjdk.java.net/browse/JDK-8193393

Webrev:
http://cr.openjdk.java.net/~egahlin/8199712.0/

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

[1] 
http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-April/031359.html


Thanks
Erik and Markus

Re: RFR(XL): 8199712: Flight Recorder

[Erik Gahlin]

Here is an updated webrev:

http://cr.openjdk.java.net/~egahlin/8199712.1/ [1]

that incorporates:

- build changes
- new event prefix, i.e. "com.oracle.jdk.CPULoad" becomes "jdk.CPULoad"
- obsolete command line options EnableTracing and UseLockedTracing
- fixed typos in the Javadoc
- simplified #include files

RFEs have been filed for other issues, CSR is approved and tests pass.

Erik and Markus

[1] Parent:

changeset:   50092:0e42d3120e51

user:clanger
date:Sat May 12 10:26:42 2018 +0200
summary: 8202915: [JAXP] Performance enhancements and cleanups in 
com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator




Greetings,

Could I have a review of 8199712: Flight Recorder

As mentioned in the preview [1] the tracing backend has been removed. 
Event metadata has been consolidated into a single XML file and event 
classes are now generated by GenerateJfrFiles.java.


Tests have been run on Linux-x64, Windows-x64 and MaxOSX-x64.

For details about the feature, see the JEP:
https://bugs.openjdk.java.net/browse/JDK-8193393

Webrev:
http://cr.openjdk.java.net/~egahlin/8199712.0/

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

[1] 
http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-April/031359.html


Thanks
Erik and Markus

Re: RFR(XL): 8199712: Flight Recorder

[Erik Gahlin]

Hi Magnus,

Sorry about not including build-dev.  See comments inline.



On 2018-04-25 13:06, Erik Gahlin wrote:

Greetings,

Could I have a review of 8199712: Flight Recorder

As mentioned in the preview [1] the tracing backend has been removed. 
Event metadata has been consolidated into a single XML file and event 
classes are now generated by GenerateJfrFiles.java.


Tests have been run on Linux-x64, Windows-x64 and MaxOSX-x64.

For details about the feature, see the JEP:
https://bugs.openjdk.java.net/browse/JDK-8193393

Webrev:
http://cr.openjdk.java.net/~egahlin/8199712.0/

Hi Erik,

When posting build changes (files in "make/*"), please always include 
build-dev. Thanks! :-)


This review is for build changes only.

* make/RunTestsPrebuiltSpec.gmk: This file has only a copyright year 
change. It can be reverted.



Will fix.


* make/copy/Copy-jdk.jfr.gmk:
COPY_HOTSPOT_TRACE_FILES should be renamed to something like 
COPY_JFR_METADATA.



Will fix.


The second part is better off rewritten using SetupCopyFiles.

Something like this, written on-the-fly, so might not work. (Email me 
privately if it does not work and you need help.)


JFR_CONF_DIR := $(TOPDIR)/src/jdk.jfr/share/conf/jfr

$(eval $(call SetupCopyFiles, COPY_JFR_CONF, \
  DEST := $(LIB_DST_DIR)/jfr, \
  FILES := $(wildcard $(JFR_CONF_DIR)/*.jfc), \
  FLATTEN := true, \
))

TARGETS += $(COPY_JFR_CONF)


Will look into it.


* make/autoconf/hotspot.m4:
It's a bit unfortunate with the --enable-jfr option. Ideally, this 
should be handled only as a JVM feature (--with-jvm-features=jfr). Try 
this piece of code instead: (Not tested, but should work.)


diff --git a/make/autoconf/hotspot.m4 b/make/autoconf/hotspot.m4
--- a/make/autoconf/hotspot.m4
+++ b/make/autoconf/hotspot.m4
@@ -26,7 +26,7 @@
 # All valid JVM features, regardless of platform
 VALID_JVM_FEATURES="compiler1 compiler2 zero minimal dtrace jvmti 
jvmci \

 graal vm-structs jni-check services management all-gcs nmt cds \
-static-build link-time-opt aot"
+static-build link-time-opt aot jfr"

 # All valid JVM variants
 VALID_JVM_VARIANTS="server client minimal core zero custom"
@@ -313,6 +313,11 @@
 AC_MSG_ERROR([Specified JVM feature 'vm-structs' requires feature 
'all-gcs'])

   fi

+  # Enable JFR by default, except on linux-sparcv9 and on minimal.
+  if test "x$OPENJDK_TARGET_OS" != xlinux || test 
"x$OPENJDK_TARGET_CPU" != xsparcv9; then

+NON_MINIMAL_FEATURES="$NON_MINIMAL_FEATURES jfr"
+  fi
+
   # Turn on additional features based on other parts of configure
   if test "x$INCLUDE_DTRACE" = "xtrue"; then
 JVM_FEATURES="$JVM_FEATURES dtrace"


Will try it.

Thanks
Erik


Otherwise, build changes look good!

/Magnus




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

[1] 
http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-April/031359.html


Thanks
Erik and Markus

Re: (8u) RFR: 8152000: Java FlightRecorder does not run on Java 8 compact 3 profile since u65

[Erik Gahlin]

Looks good

Erik

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

webrev: http://cr.openjdk.java.net/~dholmes/8152000/webrev/

This is a small adjustment to the build of compact profiles to allow a 
useful but unsupported configuration to continue to work after it 
stopped working in 8u60. We simply move the 
jdk.internal.instrumentation package from the full JRE to compact 3. 
This adds just under 41KB (21KB compressed) to rt.jar in compact 3.


Thanks,
David

Re: RFR: JDK-8166202: Tracefile gensrc cannot handle closed src dir in different location

[Erik Gahlin]

Looks good.

Erik

The xml/xsl source files used to generate sources for trace in hotspot 
have hard coded relative paths in them that make assumptions on where 
the oracle closed files are located. The source files should not make 
these kind of assumptions since that makes it difficult to change the 
repository layout structure.


While this is mostly an oracle internal change, there are some light 
changes in the open makefile too. The end result should be unchanged.


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

Webrev: http://cr.openjdk.java.net/~erikj/8166202/webrev.01/

/Erik

Re: RFR: JDK-8075056 Remove Version.java.template from jconsole

[Erik Gahlin]

Looks good, jconsole now compile in Eclipse!

Erik

Staffan Larsen skrev 2015-03-12 13:54:
The build for jconsole currently takes a template file and inserts the 
version number of the build into the file. We can simplify this by 
removing the template file and reading the java.runtime.version system 
property at runtime.


bug: https://bugs.openjdk.java.net/browse/JDK-8075056
webrev: http://cr.openjdk.java.net/~sla/8075056/webrev.00/ 



Thanks,
/Staffan

Re: Le roi est mort, vive le roi!

[Erik Gahlin]

I know 5-10 Hotspot developers who uses Visual Studio (me included) so I 
hope it's not something that is going away.


Erik

Staffan Larsen skrev 2013-11-26 20:10:

I am in full agreement with you that an IDE is a very powerful tool and I much 
prefer to work in an IDE than outside one. However, I do not do my day-to-day 
work on Windows (anymore - I used to). I don’t know how developers on Windows 
manage their work.

/Staffan

On 26 nov 2013, at 18:34, Francis ANDRE  
wrote:


Hi Staffan
Le 25/11/2013 08:53, Staffan Larsen a écrit :



The top level make produces a full jdk here: 
Z:\JDK\jdk8\build\windows-x86-normal-server-release\images\j2sdk-image
The hotspot make produces a full jdk here : 
Z:\JDK\jdk8\hotspot\build\windows\jdk-windows-i586
The hotspot create.bat produces a JVM here: 
Z:\JDK\jdk8\hotspot\build\vs-i486\compiler1

create.bat has only one parameter: the bootstrap jdk and thus does not take 
into account the configure-based top-level files. Moreover when using an 
official public jdk7 release as the jdk1.7.0_45 as the bootstrap jdk of jdk8, 
the produced jvm.vxproj does not work as the changeset associated with the JDK 
8016019 are not back ported to the jdk1.7.0_45 release. So I am wondering what 
specific parameters hotspot developers are using to test their changes under 
WXP/W7 for the jdk8 hotspot?

The visual studio projects are not the official way to build hotspot. They are 
provided as a nice-to-have feature that developers on windows maintain. The 
official way to build on all platforms is to use the makefiles.

Staffan, while the official way to build on all platforms is to use the 
makefiles, it does not make sense when talking about debugging, testing and 
developing a major, large, multi layered C++ application like hotspot.

To make it short, VisualStudio C++ is to c++ what Netbeans or Eclipse are for 
Java, a mandatory  powerful IDE for developping large and complex application. 
Can you image to code, develop, debug, test a multi-threaded java application 
fo 250 kilo lines without Netbeans, just with vi, makefiles and some additional 
tools... I do not think so... So, it is the same for hotspot on WXP/W7!

Thus, if as I understand well, WXP/W7/ X86/X64 are strategic platfoms for Oracle in 
providing a fully certified and supported Java environment, then the nice-to-have feature 
is not at level of the mission. The generation of the jvm.vxproj cannot be a second level 
citizen but should be as good as the generation of the jdk, fully maintained and 
integrated as new features, way fo building "the new build system" and so on 
are added, modified or deleted

Francis


/Staffan




Francis

David
-


Francis

the jdk8 build system, but what is the old one?

Francis

Le 18/11/2013 19:02, Magnus Ihse Bursie a écrit :

So, finally it has happened. The old build system is now removed.
The patch was just submitted to the tl forest, from where it will
move to master in a few days.

If you have personal scripts that are using hard-coded makefile
references, you might need to update them.

Thanks to everyone who helped this major project become reality:
Fredrik, Erik, Kelly, Tim, Jonas, ...

/Magnus

RFR: 2228582: Licensee source bundle tries to compile JFR

[Erik Gahlin]

Hi,

Could you please review this fix for licensee source bundle. This is a 
forward port of:


https://bugs.openjdk.java.net/browse/JDK-8001697

I removed:

ifndef OPENJDK
ifndef JAVASE_EMBEDDED

and only check for BUILD_JFR. Not sure if it is correct? I tried to 
build with:


make newbuild=true source

and I didn't get any errors on standard out, but I'm not sure how 
interpret the result. It would be good if someone more experienced with 
licensee builds could verify the results before checking in.


Thanks
Erik

Webrev:
http://cr.openjdk.java.net/~egahlin/2228582/

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