RFR: 8343057: JFR: Sorting in 'jfr view' can violate contract
Could I have a review of a change that improves the comparator used with 'jfr view'? Testing: jdk/jdk/jfr + comparison of output from 'jfr view all-views' before and after the change Thanks Erik ------------- Commit messages: - Fix trailing whitespaces - Fix trailing whitespaces - Initial Changes: https://git.openjdk.org/jdk/pull/21713/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21713&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8343057 Stats: 55 lines in 1 file changed: 49 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/21713.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21713/head:pull/21713 PR: https://git.openjdk.org/jdk/pull/21713
Could I have a review of a change that improves the comparator used with 'jfr view'?
Testing: jdk/jdk/jfr + comparison of output from 'jfr view all-views' before and after the change
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision: Update ------------- Changes: - all: https://git.openjdk.org/jdk/pull/21713/files - new: https://git.openjdk.org/jdk/pull/21713/files/5721a0f7..d93bf897 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21713&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21713&range=00-01 Stats: 20 lines in 1 file changed: 16 ins; 1 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/21713.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21713/head:pull/21713 PR: https://git.openjdk.org/jdk/pull/21713
On Tue, 29 Oct 2024 22:54:24 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of a change that improves the comparator used with 'jfr view'?
Testing: jdk/jdk/jfr + comparison of output from 'jfr view all-views' before and after the change
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Update
src/jdk.jfr/share/classes/jdk/jfr/internal/query/TableSorter.java line 74:
72: 73: private static int compareObjects(Object a, Object b) { 74: if (a == b) {
What case is most likely? Is null == null a valid comparison? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21713#discussion_r1818075461
On Sun, 27 Oct 2024 12:12:02 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Update
src/jdk.jfr/share/classes/jdk/jfr/internal/query/TableSorter.java line 74:
72: 73: private static int compareObjects(Object a, Object b) { 74: if (a == b) {
What case is most likely? Is null == null a valid comparison?
It's valid, I like to remove the case a == b first to simplify logic if only one value is null. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21713#discussion_r1824795871
On Tue, 29 Oct 2024 22:54:24 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of a change that improves the comparator used with 'jfr view'?
Testing: jdk/jdk/jfr + comparison of output from 'jfr view all-views' before and after the change
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Update
Marked as reviewed by mgronlun (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/21713#pullrequestreview-2408493214
On Fri, 25 Oct 2024 13:29:21 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Could I have a review of a change that improves the comparator used with 'jfr view'?
Testing: jdk/jdk/jfr + comparison of output from 'jfr view all-views' before and after the change
Thanks Erik
This pull request has now been integrated. Changeset: 7ad3ef7f Author: Erik Gahlin <egahlin@openjdk.org> URL: https://git.openjdk.org/jdk/commit/7ad3ef7f763ab909d7b43dbdc8f445c101a9e989 Stats: 70 lines in 1 file changed: 64 ins; 0 del; 6 mod 8343057: JFR: Sorting in 'jfr view' can violate contract Reviewed-by: mgronlun ------------- PR: https://git.openjdk.org/jdk/pull/21713
participants (2)
-
Erik Gahlin
-
Markus Grönlund