RFR: 8328572: JFR: Use Class.forPrimitiveName(String)
Could I have a review of change that use Class.forPrimitiveName(String) instead checking against every primitive type. Testing: jdk/jdk/jfr Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/19235/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19235&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8328572 Stats: 73 lines in 1 file changed: 13 ins; 56 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/19235.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19235/head:pull/19235 PR: https://git.openjdk.org/jdk/pull/19235
On Tue, 14 May 2024 17:58:36 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of change that use Class.forPrimitiveName(String) instead checking against every primitive type.
Testing: jdk/jdk/jfr
Thanks Erik
src/jdk.jfr/share/classes/jdk/jfr/internal/util/Utils.java line 253:
251: Object array = Array.newInstance(componentType, length); 252: for (int index = 0; index < length; index++) { 253: Array.set(array, index, values.get(index));
Note that `Array.set` is slow (see [JDK-8051447](https://bugs.openjdk.org/browse/JDK-8051447)); yet this woudld be fine if this is not on hot code paths. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19235#discussion_r1600777045
On Tue, 14 May 2024 23:29:06 GMT, Chen Liang <liach@openjdk.org> wrote:
Could I have a review of change that use Class.forPrimitiveName(String) instead checking against every primitive type.
Testing: jdk/jdk/jfr
Thanks Erik
src/jdk.jfr/share/classes/jdk/jfr/internal/util/Utils.java line 253:
251: Object array = Array.newInstance(componentType, length); 252: for (int index = 0; index < length; index++) { 253: Array.set(array, index, values.get(index));
Note that `Array.set` is slow (see [JDK-8051447](https://bugs.openjdk.org/browse/JDK-8051447)); yet this woudld be fine if this is not on hot code paths.
It happens when a recording file is parsed, typically the Category annotation, which means about 150-200 invocation per file. Don't think it matters. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19235#discussion_r1603223963
On Thu, 16 May 2024 12:10:17 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
src/jdk.jfr/share/classes/jdk/jfr/internal/util/Utils.java line 253:
251: Object array = Array.newInstance(componentType, length); 252: for (int index = 0; index < length; index++) { 253: Array.set(array, index, values.get(index));
Note that `Array.set` is slow (see [JDK-8051447](https://bugs.openjdk.org/browse/JDK-8051447)); yet this woudld be fine if this is not on hot code paths.
It happens when a recording file is parsed, typically the Category annotation, which means about 150-200 invocation per file. Don't think it matters.
Indeed, that sounds all good. If this becomes a performance issue, it should be fixed from java.lang.reflect instead. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19235#discussion_r1603305660
On Tue, 14 May 2024 17:58:36 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of change that use Class.forPrimitiveName(String) instead checking against every primitive type.
Testing: jdk/jdk/jfr
Thanks Erik
Marked as reviewed by mgronlun (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/19235#pullrequestreview-2063513581
On Tue, 14 May 2024 17:58:36 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of change that use Class.forPrimitiveName(String) instead checking against every primitive type.
Testing: jdk/jdk/jfr
Thanks Erik
This pull request has now been integrated. Changeset: b7ae0ae1 Author: Erik Gahlin <egahlin@openjdk.org> URL: https://git.openjdk.org/jdk/commit/b7ae0ae1d7481e66a07f40bf01c5614fdf44c2ed Stats: 73 lines in 1 file changed: 13 ins; 56 del; 4 mod 8328572: JFR: Use Class.forPrimitiveName(String) Reviewed-by: mgronlun ------------- PR: https://git.openjdk.org/jdk/pull/19235
participants (3)
-
Chen Liang
-
Erik Gahlin
-
Markus Grönlund