Re: RFR: 8287008: Improve tests for thread dumps in JSON format [v2]

2022-05-20 Thread Daniel Fuchs
On Fri, 20 May 2022 11:32:14 GMT, Alan Bateman wrote: >> This is a test-only change to add some test infrastructure and improve the >> testing of thread dumps in JSON format. The new tests added by JEP 425 for >> this thread dump format search the JSON text for strings but don't parse it >> co

Re: RFR: 8284209: Replace remaining usages of 'a the' in source code

2022-05-18 Thread Daniel Fuchs
On Wed, 18 May 2022 14:46:42 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > It's the last issue in the series, and it still touches different areas of > the code. Logging/JNDI OK - Marked as revi

Re: RFR: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable [v2]

2022-05-10 Thread Daniel Fuchs
On Tue, 10 May 2022 11:31:16 GMT, Andrey Turbanov wrote: >> src/java.desktop/share/classes/javax/imageio/spi/ServiceRegistry.java line >> 230: >> >>> 228: List l = new ArrayList<>(); >>> 229: for (Class c : categoryMap.keySet()) { >>> 230: if (c.isInstance(provider))

Re: RFR: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable [v2]

2022-05-10 Thread Daniel Fuchs
On Thu, 31 Mar 2022 08:03:23 GMT, Andrey Turbanov wrote: >> Method `Class.isAssignableFrom` is often used in form of: >> >> if (clazz.isAssignableFrom(obj.getClass())) { >> Such condition could be simplified to more shorter and performarnt code >> >> if (clazz.isInstance(obj)) { >>

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]

2022-04-28 Thread Daniel Fuchs
On Thu, 28 Apr 2022 01:34:19 GMT, Joe Darcy wrote: >> To enable more complete doclint checking (courtesy @jonathan-gibbons), >> please review this PR to add type-level @param tags where they are missing. >> >> To the maintainers of java.util.concurrent, those changes could be separated >> out

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

2022-04-27 Thread Daniel Fuchs
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the

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

2022-04-27 Thread Daniel Fuchs
On Tue, 26 Apr 2022 17:27:35 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the

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

2022-04-27 Thread Daniel Fuchs
On Tue, 26 Apr 2022 17:27:35 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces

2022-04-27 Thread Daniel Fuchs
On Tue, 26 Apr 2022 22:24:26 GMT, Joe Darcy wrote: > To enable more complete doclint checking (courtesy @jonathan-gibbons), please > review this PR to add type-level @param tags where they are missing. > > To the maintainers of java.util.concurrent, those changes could be separated > out in an

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

2022-04-26 Thread Daniel Fuchs
On Tue, 26 Apr 2022 17:27:35 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the

Re: RFR: 8285366: Fix typos in serviceability

2022-04-21 Thread Daniel Fuchs
On Thu, 21 Apr 2022 14:03:39 GMT, Daniel Fuchs wrote: >> I ran `codespell` on modules owned by the serviceability team >> (`java.instrument java.management.rmi java.management jdk.attach >> jdk.hotspot.agent jdk.internal.jvmstat jdk.jcmd jdk.jconsole jdk.jdi >> j

Re: RFR: 8285366: Fix typos in serviceability

2022-04-21 Thread Daniel Fuchs
On Thu, 21 Apr 2022 11:22:48 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by the serviceability team > (`java.instrument java.management.rmi java.management jdk.attach > jdk.hotspot.agent jdk.internal.jvmstat jdk.jcmd jdk.jconsole jdk.jdi > jdk.jdwp.agent jdk.jstatd jdk.

Re: RFR: 8284161: Implementation of Virtual Threads (Preview)

2022-04-13 Thread Daniel Fuchs
On Tue, 12 Apr 2022 13:02:44 GMT, Alan Bateman wrote: >> src/java.base/share/classes/java/io/BufferedReader.java line 101: >> >>> 99: */ >>> 100: public BufferedReader(Reader in, int sz) { >>> 101: Objects.requireNonNull(in); >> >> Not sure if that even matters - but there will

Re: RFR: 8284161: Implementation of Virtual Threads (Preview)

2022-04-11 Thread Daniel Fuchs
On Fri, 8 Apr 2022 13:43:39 GMT, Alan Bateman wrote: > This is the implementation of JEP 425: Virtual Threads (Preview); TBD which > JDK version to target. > > We will refresh this PR periodically to pick up changes and fixes from the > loom repo. > > Most of the new mechanisms in the HotSpot

Re: RFR: 8283254: Remove redundant class jdk/internal/agent/spi/AgentProvider

2022-03-23 Thread Daniel Fuchs
On Tue, 22 Mar 2022 21:19:20 GMT, Kevin Walls wrote: > There are no uses of jdk/internal/agent/spi/AgentProvider, since the SNMP > agent was removed ( 8071367 ): this class should be removed. It is not a > public interface. > > Remove > src/jdk.management.agent/share/classes/jdk/internal/age

Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation

2022-03-16 Thread Daniel Fuchs
On Tue, 15 Mar 2022 20:22:16 GMT, Kevin Walls wrote: > Removing permission checks which, in the presence of a Security Manager, > would check for a RuntimePermission "className.subclass". This was to > prevent subclassing these classes, but is no longer necessary with strong > encapsulation f

Re: RFR: 8280400: JDK 19 L10n resource files update - msgdrop 10

2022-03-10 Thread Daniel Fuchs
On Thu, 10 Mar 2022 17:00:09 GMT, Naoto Sato wrote: > IIRC, localized resource files should have the same copyright year as the > base English one. That's what I was told by the l10n engineer when I had the > same comment. Thanks Naoto! I have no objection then. - PR: https://git

Re: RFR: 8280400: JDK 19 L10n resource files update - msgdrop 10

2022-03-10 Thread Daniel Fuchs
On Wed, 9 Mar 2022 21:09:30 GMT, Alisen Chung wrote: > msg drop for jdk19, Mar 9, 2022 For simple webserver resource files - should the copyright year be 2022? - PR: https://git.openjdk.java.net/jdk/pull/7765

Re: RFR: 6779701: Wrong defect ID in the code of test LocalRMIServerSocketFactoryTest.java

2022-02-07 Thread Daniel Fuchs
On Mon, 7 Feb 2022 17:28:29 GMT, Kevin Walls wrote: > Trivial comment and exception text update, correcting a bug ID to make more > sense. LGTM - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7371

Re: RFR: 8076089: Cleanup: Inline & remove sun.management.Util.newException

2022-01-26 Thread Daniel Fuchs
On Wed, 26 Jan 2022 04:35:58 GMT, Jaikiran Pai wrote: > Can I please get a review for this cleanup that's requested in > https://bugs.openjdk.java.net/browse/JDK-8076089? > > The change here removes a package private method > `sun.management.Util.newException(Exception e)` and inlines its > i

Re: RFR: JDK-8280492: Address remaining doclint issues in JDK build

2022-01-24 Thread Daniel Fuchs
On Sat, 22 Jan 2022 21:09:03 GMT, Joe Darcy wrote: > Use presumed syntax that will be introduced by JDK-8280488. Marked as reviewed by dfuchs (Reviewer). LGTM. I hope in the future IDEs will pick that rule up and offer some help when writing `{@link }` `@see`... - PR: https://git

Re: RFR: 8280010: Remove double buffering of InputStream for Properties.load

2022-01-14 Thread Daniel Fuchs
On Mon, 10 Jan 2022 20:46:36 GMT, Andrey Turbanov wrote: > `Properties.load` uses `java.util.Properties.LineReader`. LineReader already > buffers input stream. Hence wrapping InputStream in BufferedInputStream is > redundant. Changes to `java.util.logging` look fine. - PR: https:

Re: RFR: 8275322: Change nested classes in java.management to static nested classes

2021-10-15 Thread Daniel Fuchs
On Fri, 15 Oct 2021 06:43:13 GMT, Andrey Turbanov wrote: > Non-static classes hold a link to their parent classes, which can be avoided. Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5962

Re: RFR: 8274318: Replace 'for' cycles with iterator with enhanced-for in java.management [v2]

2021-10-06 Thread Daniel Fuchs
On Fri, 1 Oct 2021 21:12:57 GMT, Andrey Turbanov wrote: >> There are a few places in code, where manual `for` loop is used with >> Iterator to iterate over Collection. >> Instead of manual `for` cycles, it's preferred to use enhanced-for cycle >> instead: it's less verbose, makes code easier t

Re: RFR: 8274757: Cleanup unnecessary calls to Throwable.initCause() in java.management module

2021-10-05 Thread Daniel Fuchs
On Thu, 16 Sep 2021 20:45:36 GMT, Andrey Turbanov wrote: > Pass cause exception as constructor parameter is shorter and easier to read. Nice simplification. - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5552

Re: RFR: 8274464: Remove redundant stream() call before forEach in java.* modules

2021-09-29 Thread Daniel Fuchs
On Wed, 15 Sep 2021 07:12:25 GMT, Andrey Turbanov wrote: > 8274464: Remove redundant stream() call before forEach in java.* modules LGTM - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5520

Re: RFR: 8274168: Avoid String.compareTo == 0 to check String equality in java.management [v2]

2021-09-23 Thread Daniel Fuchs
On Thu, 23 Sep 2021 08:00:25 GMT, Andrey Turbanov wrote: >> Cleanup places, where String.compareTo is used to check String's equality. >> Instead String.equals or switch expression could be used. They are faster >> and code is cleaner. > > Andrey Turbanov has updated the pull request incrementa

Re: RFR: 8272805: Avoid looking up standard charsets [v2]

2021-08-23 Thread Daniel Fuchs
On Sun, 22 Aug 2021 23:02:06 GMT, Sergey Bylokhov wrote: >> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120. >> >> In many places standard charsets are looked up via their names, for example: >> absolutePath.getBytes("UTF-8"); >> >> This could be done more efficiently(up t

Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules

2021-08-11 Thread Daniel Fuchs
On Tue, 10 Aug 2021 05:08:54 GMT, Sergey Bylokhov wrote: > This is the continuation of JDK-8233884 and JDK-8271456. This change affects > fewer cases so I fix all "java." modules at once. > > In many places standard charsets are looked up via their names, for example: > absolutePath.getBytes("U

Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules

2021-08-11 Thread Daniel Fuchs
On Tue, 10 Aug 2021 05:08:54 GMT, Sergey Bylokhov wrote: > This is the continuation of JDK-8233884 and JDK-8271456. This change affects > fewer cases so I fix all "java." modules at once. > > In many places standard charsets are looked up via their names, for example: > absolutePath.getBytes("U

Re: [jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K [v2]

2021-06-28 Thread Daniel Fuchs
On Sat, 26 Jun 2021 23:55:46 GMT, Weijun Wang wrote: >> src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java >> line 53: >> >>> 51: private static final long CURRENT_PID = >>> AccessController.doPrivileged( >>> 52: (PrivilegedAction) >>> ProcessHandle::curr

Re: RFR: 8253119: Remove the legacy PlainSocketImpl and PlainDatagramSocketImpl implementation [v5]

2021-06-25 Thread Daniel Fuchs
On Fri, 25 Jun 2021 11:48:52 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my changes for the removal of the legacy >> `PlainSocketImpl` and `PlainDatagramSocketImpl` implementations? >> >> In JDK 13, JEP 353 provided a drop in replacement for the legacy >> `PlainSoc

Re: RFR: 8253119: Remove the legacy PlainSocketImpl and PlainDatagramSocketImpl implementation [v4]

2021-06-25 Thread Daniel Fuchs
On Fri, 25 Jun 2021 10:56:15 GMT, Alan Bateman wrote: >> Patrick Concannon 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 five additional >> co

Re: RFR: 8253119: Remove the legacy PlainSocketImpl and PlainDatagramSocketImpl implementation [v4]

2021-06-25 Thread Daniel Fuchs
On Fri, 25 Jun 2021 10:25:52 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my changes for the removal of the legacy >> `PlainSocketImpl` and `PlainDatagramSocketImpl` implementations? >> >> In JDK 13, JEP 353 provided a drop in replacement for the legacy >> `PlainSoc

Re: RFR: 8253119: Remove the legacy PlainSocketImpl and PlainDatagramSocketImpl implementation [v3]

2021-06-24 Thread Daniel Fuchs
On Thu, 24 Jun 2021 11:44:19 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my changes for the removal of the legacy >> `PlainSocketImpl` and `PlainDatagramSocketImpl` implementations? >> >> In JDK 13, JEP 353 provided a drop in replacement for the legacy >> `PlainSoc

Re: RFR: 8253119: Remove the legacy PlainSocketImpl and PlainDatagramSocketImpl implementation [v3]

2021-06-24 Thread Daniel Fuchs
On Thu, 24 Jun 2021 14:22:21 GMT, Alan Bateman wrote: >> I've created an issue to track this: >> https://bugs.openjdk.java.net/browse/JDK-8269288 > >> I've created an issue to track this: >> https://bugs.openjdk.java.net/browse/JDK-8269288 > > Thanks. So are you keeping the Objects.requireNonN

Re: RFR: 8253119: Remove the legacy PlainSocketImpl and PlainDatagramSocketImpl implementation

2021-06-23 Thread Daniel Fuchs
On Wed, 23 Jun 2021 14:32:22 GMT, Alan Bateman wrote: >> Yes - the constructor would throw NPE - but there's no need to call this >> constructor if `impl` is null here - it's better to fail early. > > This is probably a case where the spec needs say that an error (maybe an > unspecified error)

Re: RFR: 8253119: Remove the legacy PlainSocketImpl and PlainDatagramSocketImpl implementation

2021-06-23 Thread Daniel Fuchs
On Wed, 23 Jun 2021 13:05:24 GMT, Alan Bateman wrote: >> Hi, >> >> Could someone please review my changes for the removal of the legacy >> `PlainSocketImpl` and `PlainDatagramSocketImpl` implementations? >> >> In JDK 13, JEP 353 provided a drop in replacement for the legacy >> `PlainSocketImp

Re: RFR: Merge jdk17 [v2]

2021-06-14 Thread Daniel Fuchs
On Mon, 14 Jun 2021 15:58:15 GMT, Jesper Wilhelmsson wrote: >> Forwardport JDK 17 -> JDK 18 > > Jesper Wilhelmsson 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. Looked a

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v3]

2021-05-24 Thread Daniel Fuchs
On Fri, 21 May 2021 20:37:30 GMT, Weijun Wang wrote: >> The code change refactors classes that have a `SuppressWarnings("removal")` >> annotation that covers more than 50KB of code. The big annotation is often >> quite faraway from the actual deprecated API usage it is suppressing, and >> with

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v2]

2021-05-21 Thread Daniel Fuchs
On Fri, 21 May 2021 14:00:39 GMT, Weijun Wang wrote: >> The code change refactors classes that have a `SuppressWarnings("removal")` >> annotation that covers more than 50KB of code. The big annotation is often >> quite faraway from the actual deprecated API usage it is suppressing, and >> with

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

2021-05-21 Thread Daniel Fuchs
On Wed, 19 May 2021 13:47:53 GMT, Weijun Wang wrote: >> Please review this implementation of [JEP >> 411](https://openjdk.java.net/jeps/411). >> >> The code change is divided into 3 commits. Please review them one by one. >> >> 1. >> https://github.com/openjdk/jdk/commit/576161d15423f58281e38

Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager

2021-05-18 Thread Daniel Fuchs
On Mon, 17 May 2021 17:51:36 GMT, Weijun Wang wrote: > Please review the test changes for [JEP > 411](https://openjdk.java.net/jeps/411). > > With JEP 411 and the default value of `-Djava.security.manager` becoming > `disallow`, tests calling `System.setSecurityManager()` need > `-Djava.secu

Re: RFR: 8266567: Fix javadoc tag references in sun.management.jmxremote.ConnectorBootstrap [v4]

2021-05-12 Thread Daniel Fuchs
On Wed, 12 May 2021 09:14:53 GMT, Pavel Rappo wrote: >> This fixes two javadoc tag references and several typos. References are >> fixed by removing whitespace before the opening `(`. That whitespace caused >> the opening `(` and the rest of the reference to be parsed as a link label. >> >> Si

Re: RFR: 8266567: Fix javadoc tag references in sun.management.jmxremote.ConnectorBootstrap [v2]

2021-05-05 Thread Daniel Fuchs
On Wed, 5 May 2021 18:39:14 GMT, Pavel Rappo wrote: >> This fixes two javadoc tag references and several typos. References are >> fixed by removing whitespace before the opening `(`. That whitespace caused >> the opening `(` and the rest of the reference to be parsed as a link label. >> >> Sin

Re: RFR: 8266567: Fix javadoc tag references in sun.management.jmxremote.ConnectorBootstrap

2021-05-05 Thread Daniel Fuchs
On Wed, 5 May 2021 15:59:43 GMT, Pavel Rappo wrote: > This fixes two javadoc tag references and several typos. References are fixed > by removing whitespace before the opening `(`. That whitespace caused the > opening `(` and the rest of the reference to be parsed as a link label. > > Since we

Integrated: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records

2021-04-12 Thread Daniel Fuchs
On Thu, 25 Mar 2021 17:30:52 GMT, Daniel Fuchs wrote: > This RFE proposes to extend the MXBean framework to define a mapping to > records. > > The MXBean framework already defines a mapping of `CompositeType` to plain > java objects. Records are a natural representation of Co

Re: RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records [v6]

2021-04-08 Thread Daniel Fuchs
On Wed, 31 Mar 2021 21:43:58 GMT, Mandy Chung wrote: >> Daniel Fuchs has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - minor style issue >> - minor style issue > > Thanks for making the chang

Re: RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records [v7]

2021-04-02 Thread Daniel Fuchs
{return name;} > @ConstructorParameters({"number", "name"}) > public NamedNumber(int number, String name) { > this.number = number; > this.name = name; > } > private final int number; > private final String name; > } > &

Re: RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records [v6]

2021-03-31 Thread Daniel Fuchs
{return name;} > @ConstructorParameters({"number", "name"}) > public NamedNumber(int number, String name) { > this.number = number; > this.name = name; > } > private final int number; > private final String name; > } > >

Re: RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records [v2]

2021-03-31 Thread Daniel Fuchs
On Wed, 31 Mar 2021 18:32:31 GMT, Mandy Chung wrote: >> Hi Mandy, I have updated the PR with a revised version of the text above. Is >> that more in line what you had in mind? >> >> best regards, >> -- daniel > > This looks better. I like a separate mapping section for records since the > map

Re: RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records [v5]

2021-03-31 Thread Daniel Fuchs
{return name;} > @ConstructorParameters({"number", "name"}) > public NamedNumber(int number, String name) { > this.number = number; > this.name = name; > } > private final int number; > private final String name; > } > >

Re: RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records [v4]

2021-03-31 Thread Daniel Fuchs
{return name;} > @ConstructorParameters({"number", "name"}) > public NamedNumber(int number, String name) { > this.number = number; > this.name = name; > } > private final int number; > private final String name; > } > &g

Re: RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records [v2]

2021-03-31 Thread Daniel Fuchs
On Mon, 29 Mar 2021 22:12:01 GMT, Daniel Fuchs wrote: >> You add record in "Mappings for other types". I think it deserves a >> separate section "Mappings for record classes" (maybe after primitive >> types). It's useful to add a row for record i

Re: RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records [v3]

2021-03-31 Thread Daniel Fuchs
{return name;} > @ConstructorParameters({"number", "name"}) > public NamedNumber(int number, String name) { > this.number = number; > this.name = name; > } > private final int number; > private final String name; > } > &

Re: RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records [v2]

2021-03-29 Thread Daniel Fuchs
On Sat, 27 Mar 2021 01:49:38 GMT, Mandy Chung wrote: >> src/java.management/share/classes/javax/management/MXBean.java line 757: >> >>> 755: >>> 756: If the class is a {@link Record}, its getters are the >>> 757: accessors for the record components. Otherwise, the >> >> It may be goo

Re: RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records [v2]

2021-03-26 Thread Daniel Fuchs
{return name;} > @ConstructorParameters({"number", "name"}) > public NamedNumber(int number, String name) { > this.number = number; > this.name = name; > } > private final int number; > private final String name; > } > &g

Re: RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records [v2]

2021-03-26 Thread Daniel Fuchs
On Fri, 26 Mar 2021 14:45:05 GMT, Alan Bateman wrote: >> Daniel Fuchs has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed typo. Moved example with record after example with from method to >> respect t

RFR: 8264124: Update MXBean specification and implementation to extend mapping of CompositeType to records

2021-03-25 Thread Daniel Fuchs
This RFE proposes to extend the MXBean framework to define a mapping to records. The MXBean framework already defines a mapping of `CompositeType` to plain java objects. Records are a natural representation of CompositeTypes. A record can be easily reconstructed from a `CompositeData` through th

Re: RFR: 8263855: Use the blessed modifier order in java.management/naming [v2]

2021-03-22 Thread Daniel Fuchs
On Fri, 19 Mar 2021 17:13:58 GMT, Alex Blewitt wrote: >> As with #2993 changing the order of `final static` to `static final` for the >> `java.management`, `java.management.rmi` and `java.naming` modules. > > Alex Blewitt has updated the pull request incrementally with one additional > commit

Re: RFR: 8262001: java/lang/management/ThreadMXBean/ResetPeakThreadCount.java failed with "RuntimeException: Current Peak = 14 Expected to be == previous peak = 7 + 8"

2021-03-09 Thread Daniel Fuchs
On Tue, 9 Mar 2021 01:31:33 GMT, Alex Menkov wrote: > The fix updates ResetPeakThreadCount.java test - increases number of threads, > but relaxes conditions so start/termination of system threads don't cause > failures. > Additional changes: > - store threads in a list instead of array (we need

Re: RFR: 8224775: test/jdk/com/sun/jdi/JdwpListenTest.java failed to attach [v2]

2021-02-23 Thread Daniel Fuchs
On Mon, 22 Feb 2021 21:36:52 GMT, Alex Menkov wrote: >> The fix also partially fixes JdwpAttachTest failures (JDK-8253940). >> The failures are caused by network configuration changes during test >> execution ("global" IPv6 addresses disappears from interface). >> To minimize chances of intermit

Re: RFR: 8224775: test/jdk/com/sun/jdi/JdwpListenTest.java failed to attach

2021-02-22 Thread Daniel Fuchs
On Thu, 18 Feb 2021 21:43:00 GMT, Alex Menkov wrote: > The fix also partially fixes JdwpAttachTest failures (JDK-8253940). > The failures are caused by network configuration changes during test > execution ("global" IPv6 addresses disappears from interface). > To minimize chances of intermittent

Re: RFR: 8260448: Simplify ManagementFactory$PlatformMBeanFinder

2021-01-27 Thread Daniel Fuchs
On Tue, 26 Jan 2021 19:34:35 GMT, Claes Redestad wrote: > Simplify and desugar the static initialization of PlatformMBeanFinder. > > While arguably making the code easier to comprehend, this saves 5-6ms on > startup in various applications such as the spring-petclinic. LGTM - it's a bit sad t

Re: RFR: 8256167: Convert JDK use of `Reference::get` to `Reference::refersTo`

2020-12-04 Thread Daniel Fuchs
On Thu, 3 Dec 2020 22:54:54 GMT, Mandy Chung wrote: > This patch replaces some uses of `Reference::get` to `Reference::refersTo` to > avoid keeping a referent strongly reachable that could cause unnecessary > delay in collecting such object. I only made change in some but not all > classes i

Re: jmx-dev MBean attribute deprecation

2020-07-23 Thread Daniel Fuchs
Hi Milan, JMX is being maintained by serviceability these days. I am forwarding your question there. This is an interesting observation. JMX makes it possible to define and use annotations on methods/getters/setters, to add key/value pairs to the Descriptor of the corresponding MBeanFeatureInfo.

Re: jmx-dev RFR 8240604: Rewrite sun/management/jmxremote/bootstrap/CustomLauncherTest.java test to make binaries from source file

2020-03-05 Thread Daniel Fuchs
Hi Alexander, Fixes to JMX & management agent are reviewed on the seviceability-dev (added in to:) these days. best regards, -- daniel On 05/03/2020 13:17, Alexander Scherbatiy wrote: Hello, Could you review a small enhancement where the test CustomLauncherTest is updated to build binary la

Re: RFR 8232622: Technical debt in BadAttributeValueExpException

2020-02-13 Thread Daniel Fuchs
Hi Roger, OK - I can accept that then. best regards, -- daniel On 13/02/2020 18:18, Roger Riggs wrote: Hi Daniel, That's part of the technical debt that has to go. The change to do the conversion in the constructor has been there since JDK8. And the value of the argument is informative.

Re: RFR 8232622: Technical debt in BadAttributeValueExpException

2020-02-13 Thread Daniel Fuchs
Hi Roger, I think you will need to preserve these cases: On 13/02/2020 15:52, Roger Riggs wrote: -    || valObj instanceof Long -    || valObj instanceof Integer -    || valObj instanceof Float -    || valObj instanceof Double -    ||

Re: jmx-dev RFR(S): 8234484: Add ability to configure third port for remote JMX

2020-01-17 Thread Daniel Fuchs
Thanks Felix! On 16/01/2020 02:01, Yangfei (Felix) wrote: Modified accordingly in new webrev:http://cr.openjdk.java.net/~fyang/8234484/webrev.02/ Also renamed property to: com.sun.management.jmxremote.local.port That looks fine to me. best regards, -- daniel

Re: RFR: 8236873: Worker has a deadlock bug

2020-01-15 Thread Daniel Fuchs
Hi Daniil, That looks fine to me. best regards, -- daniel On 15/01/2020 18:15, Daniil Titov wrote: Please review a change [1] that fixes a deadlock issue [2] in sun.tools.jconsole.Worker class. There is no need in guarding "stopped" flag by a lock. The fix removes this excessive locking an

Re: jmx-dev RFR: 8213222: remove RMIConnectorServer.CREDENTIAL_TYPES

2020-01-13 Thread Daniel Fuchs
Hi Daniil, Looks good to me as well. I wonder however if the release note should point to the replacement? Something like: The terminally deprecated constant `javax.management.remote.rmi.RMIConnectorServer.CREDENTIAL_TYPE` has been removed. A filter pattern can be specified instead using `RMI

Re: jmx-dev RFR(S): 8234484: Add ability to configure third port for remote JMX

2020-01-13 Thread Daniel Fuchs
Hi Felix, On 11/01/2020 07:37, Yangfei (Felix) wrote: Hi Daniel, Thanks for the suggestions. I modified the patch making the third port also configurable via the management.properties file. New webrev: http://cr.openjdk.java.net/~fyang/8234484/webrev.01/ I think I would prefer

Re: RFR (M): 8231209: [REDO] ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

2019-09-23 Thread Daniel Fuchs
Hi, On 21/09/2019 00:28, Hohensee, Paul wrote: java.lang.management.ThreadMXBean has two default methods that throw UnsupportedOperationException: ThreadInfo[] getThreadInfo(long[], boolean, boolean, int) ThreadInfo[] dumpAllThreads(boolean, boolean, int) Without actually testing it, is it sa

Re: RFR (M): 8231209: [REDO] ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

2019-09-20 Thread Daniel Fuchs
Hi Paul, It might be worth double checking what happens if you create a MXBean proxy to access the com.sun.management.ThreadMXBean in a remote server: https://download.java.net/java/early_access/jdk14/docs/api/java.management/java/lang/management/ManagementFactory.html#getPlatformMXBean(javax.ma

Re: 8214545: sun/management/jmxremote/bootstrap tests hang in revokeall.exe on Windows

2019-05-21 Thread Daniel Fuchs
On 21/05/2019 04:25, Daniil Titov wrote: Please review un updated version of the previous change that also removes unnecessary line chmod ug+x $REVOKEALL from test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh Webrev:http://cr.openjdk.java.net/~dtitov/8214545/webrev.03

Re: jmx-dev RFR: 8214545: sun/management/jmxremote/bootstrap tests hang in revokeall.exe on Windows

2019-05-20 Thread Daniel Fuchs
Hi, On 20/05/2019 01:43, David Holmes wrote: Webrev: http://cr.openjdk.java.net/~dtitov/8214545 Bug: https://bugs.openjdk.java.net/browse/JDK-8214545 The count-- is obvious as it is the loop counter, but it is far from clear to me that i++ is correct. I don't fully understand the logic but i i

Re: RFR: JDK-8224028: loop initial declarations introduced by JDK-8184770 (jdwp)

2019-05-16 Thread Daniel Fuchs
Hi Ao Qi, I'm adding serviceability-dev, since this is for jdwp. The proposed changes look good to me - but please get someone from the serviceability team to review this. best regards, -- daniel On 16/05/2019 08:41, Ao Qi wrote: Hi, I found build is failed on CentOS 7.6, because of loop i

Re: RFR: JDK-8184770: JDWP support for IPv6

2019-04-01 Thread Daniel Fuchs
Hi Arthur, Not directly related to Alex's original question but... On 30/03/2019 00:03, Arthur Eubanks wrote: We have some ipv6 patches as well in this area as well (as well as other patches to support ipv6 only environments) that we're trying to upstream. I don't understand the code myself, b

Re: RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't

2019-03-14 Thread Daniel Fuchs
18:22, Daniel Fuchs wrote: This is what I have tried: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8219585/04/webrev/ If you think we should try this one. That's fine with me. I have now also tested webrev.04 with a test config that uses the fastdebug VM and observed many intermi

Re: RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't

2019-03-12 Thread Daniel Fuchs
Hi Severin, On 08/03/2019 13:20, Severin Gehwolf wrote: On Fri, 2019-03-08 at 11:59 +, Daniel Fuchs wrote: - please verify on your local box if testing both plain & SSL with a Thread.sleep(1000) in between makes the test more stable. If it does, then I'd prefer we go down

Re: RFR: JDK-8220257: fix headings in java.instrument

2019-03-12 Thread Daniel Fuchs
On 12/03/2019 14:24, Gary Adams wrote: Need one reviewer for this trivial correction . Having had to do the same for java.logging, I can say that the doc changes to package-info.java look good to me :-) AFAICT the changes to the copyright notice look OK too. best regards, -- daniel After

Re: RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't

2019-03-08 Thread Daniel Fuchs
-- daniel I'll run this through jdk/submit now. Could you run this through your test system too, please? Would you be OK with getting this patch pushed once we'd have positive results for both? Thanks, Severin On Fri, 2019-03-01 at 15:08 +, Daniel Fuchs wrote: Hi Severin, O

Re: RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't

2019-03-01 Thread Daniel Fuchs
Hi Severin, On 01/03/2019 15:49, Severin Gehwolf wrote: On Fri, 2019-03-01 at 15:08 +, Daniel Fuchs wrote: I ran it 50 times in our test system - and it passed on all platforms, so there's yet hope :-) Thanks for this! I take it, it actually runs the test on some of those test sy

Re: RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't

2019-03-01 Thread Daniel Fuchs
Hi Severin, On 28/02/2019 15:47, Severin Gehwolf wrote: Yes, thanks for looking at this Daniel. That was my determination as well. However, even if we do all of the above, and add a test config with /etc/hosts mapping a non-loopback address to "localhost" it would break other tests. E.g. this on

Re: RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't

2019-02-28 Thread Daniel Fuchs
Hi Severin, On 27/02/2019 16:26, Daniel Fuchs wrote: On Mac OS X and Linux - I saw it failed too - though with lower frequency (like 3 or 4 times every 100 runs), and AFAICS usually later and for a different reason: when it fails, it usually timeouts when trying to test over SSL. On Solaris

Re: RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't

2019-02-27 Thread Daniel Fuchs
On 26/02/2019 18:58, Severin Gehwolf wrote: FWIW, I use 'make test TEST="..."'. Isn't the testing system using that too? I am not sure. But I'd expect it to be similar. [...] When I was working on that patch years ago, I think one testing machine got configured in a way so that it wouldn't t

Re: RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't

2019-02-26 Thread Daniel Fuchs
Hi Severin, On 25/02/2019 15:31, Severin Gehwolf wrote: Hi Daniel, Thanks for the review! Latest webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8219585/02/webrev/ [...] I'm not sure we need a larger timeout. It's 20 seconds on my machine which seems more than enough. What makes yo

Re: RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't

2019-02-25 Thread Daniel Fuchs
Hi Severin, Thanks for looking into this! On 25/02/2019 15:31, Severin Gehwolf wrote: Hi Daniel, Thanks for the review! Latest webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8219585/02/webrev/ On Fri, 2019-02-22 at 18:31 +, Daniel Fuchs wrote: Hi Severin, Did you manage to

Re: RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't

2019-02-22 Thread Daniel Fuchs
Hi Severin, Did you manage to make the test pass locally? This test has had a long history of failing, and I've come to suspect that it might be flawed. First - it has /timeout=5. I believe that's much too short. It immediately failed the first time I ran it locally on my machine. Then it uses

Re: [RFR]8215622: Add dump to file support for jmap histo

2019-02-12 Thread Daniel Fuchs
Hi Paul, On 12/02/2019 16:27, Hohensee, Paul wrote: I don't see a way to change the Status from Closed to anything else. There's no option I can find to do so. It may be that only the assignee can see this. On the closed CSRs assigned to me I can see a "Back to Draft" button next to "More".

Re: Review Request JDK-8212795: ThreadInfoCompositeData.toCompositeData fails to map ThreadInfo to CompositeData

2018-10-25 Thread Daniel Fuchs
Hi Mandy, I agree that this looks more robust and will be better for long term maintainability. I'm just surprised that 156 static CompositeType compositeType() { 157 return STACK_TRACE_ELEMENT_COMPOSITE_TYPE; 158 } is no longer (or was never) needed in StackTraceElementCompo

Re: RFR JDK-8212197: OpenDataException thrown when constructing CompositeData for StackTraceElement

2018-10-16 Thread Daniel Fuchs
Hi Mandy, This looks good to me. best regards, -- daniel On 15/10/2018 23:02, Mandy Chung wrote: Webrev: http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8212197/webrev.00/ This simple patch fixes the issue Sven reports [1] where the order of names and values when creating CompositeData for

Re: RFR (S) 8048215: [TESTBUG] java/lang/management/ManagementFactory/ThreadMXBeanProxy.java Expected non-null LockInfo

2018-10-15 Thread Daniel Fuchs
Hi David, Good finding! Looks reasonable to me. best regards, -- daniel On 15/10/2018 05:23, David Holmes wrote: bug: https://bugs.openjdk.java.net/browse/JDK-8048215 webrev: http://cr.openjdk.java.net/~dholmes/8048215/webrev/ Simple race condition in the test. The main thread does checks th

Re: RFR : JDK-8170299 - Debugger does not stop inside the low memory notifications code

2018-08-24 Thread Daniel Fuchs
Hi Harsha, On 23/08/2018 17:35, Daniel Fuchs wrote: So all in all - maybe this is worth fixing but better early in the release than late. I also wonder whether such a behavior change should deserve a release note (or a switch to revert to the old behavior - though I do hope that isn't nece

Re: RFR : JDK-8170299 - Debugger does not stop inside the low memory notifications code

2018-08-23 Thread Daniel Fuchs
Hi Harsha, On a high level point of view, I think the fix looks good. I like the use of CopyOnWriteArrayList and removeIf. IIUC there is a single instance of ServiceThread in the VM, so using a static single thread executor to call the listeners should preserve the order in which the notification

Re: [11] RFR: 8205653: test/jdk/sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java and RmiSslBootstrapTest.sh fail with handshake_failure

2018-06-28 Thread Daniel Fuchs
[ccing serviceability-dev@openjdk.java.net] Hi Siba, This looks good to me - but I'm not a SSL expert. It would be good to get someone from the security team eyeball those changes (Xuelei? Brad?) I added serviceability-dev@openjdk.java.net in cc as this is where reviews for JMX/Monitoring chang

Re: RFR JDK-8198253: ThreadInfo.from(CompositeData) assigning fields incorrectly in JDK 9

2018-02-28 Thread Daniel Fuchs
avoid the ambiguity. * The {@code "lockedStackFrame"} attribute in * {@link MonitorInfo#from(CompositeData) MonitorInfo}'s composite type * must represent {@code StackTraceElement} of the same version N. Looks fine! -- daniel Mandy On 2/28/18 3:07 AM, Dan

Re: RFR JDK-8198253: ThreadInfo.from(CompositeData) assigning fields incorrectly in JDK 9

2018-02-28 Thread Daniel Fuchs
Hi Mandy, This looks very good. In the API documentation of ThreadInfo::from, below the table that lists the attributes of StackTraceElement, I wonder if the following text should be added for completeness: ```A CompositeData representing a MonitorInfo of version N must contain a lockedStackTr

Re: RFR JDK-8198253: ThreadInfo.from(CompositeData) assigning fields incorrectly in JDK 9

2018-02-26 Thread Daniel Fuchs
Hi Mandy, Nice to see this fixed. In ThreadInfoCompositeData::initV6CompositeType 425 if (name.equals(STACK_TRACE)) { 426 ot = new ArrayType<>(1, StackTraceElementCompositeData.v5CompositeType()); 427 } else if (name.equals(LO

  1   2   3   4   >