RFR: 8338389: [JFR] Long strings should be added to the string pool
When committing JFR events with string data longer than 128 characters, the string isn't added to the JFR string pool. In scenarios with many events containing large string values, this can lead to very large JFR recordings. This is mostly the case for custom events containing f.i. SQL strings or other data. In the case where the string data consist of mostly duplicate data, adding them to the string pool has been shown to reduce the recording size by a factor of 10. This change will add all longer strings (>128 characters) to the string pool. Strings of length 16-128 are still added using the same pre-cache methodology as before. The raised StringPool.MAX_LIMIT is used by [`jdk.jfr.internal.event.EventWriter::putString`](https://github.com/openjdk/jdk/blob/master/src/jdk.jfr/share/classes/jdk/jfr...) to determine which strings to add to the pool. ## Testing - tier1-tier3 - jdk_jfr - Internal testing with high load, using custom JFR events containing long strings have been performed ------------- Commit messages: - 8338389: [JFR] Long strings should be added to the string pool - 8338389: [JFR] Long strings should be added to the string pool Changes: https://git.openjdk.org/jdk/pull/20596/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20596&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8338389 Stats: 122 lines in 2 files changed: 118 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/20596.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20596/head:pull/20596 PR: https://git.openjdk.org/jdk/pull/20596
On Thu, 15 Aug 2024 13:16:24 GMT, Joakim Nordström <jnordstrom@openjdk.org> wrote:
When committing JFR events with string data longer than 128 characters, the string isn't added to the JFR string pool. In scenarios with many events containing large string values, this can lead to very large JFR recordings. This is mostly the case for custom events containing f.i. SQL strings or other data. In the case where the string data consist of mostly duplicate data, adding them to the string pool has been shown to reduce the recording size by a factor of 10.
This change will add all longer strings (>128 characters) to the string pool. Strings of length 16-128 are still added using the same pre-cache methodology as before. The raised StringPool.MAX_LIMIT is used by [`jdk.jfr.internal.event.EventWriter::putString`](https://github.com/openjdk/jdk/blob/master/src/jdk.jfr/share/classes/jdk/jfr...) to determine which strings to add to the pool.
## Testing - tier1-tier3 - jdk_jfr - Internal testing with high load, using custom JFR events containing long strings have been performed
Marked as reviewed by mgronlun (Reviewer). test/jdk/jdk/jfr/jvm/TestLongStringsInPool.java line 61:
59: 60: Recording firstRec = new Recording(); 61: firstRec.enable(StringEvent.class);
No need to explicitly enable user-define events. They are turned on by default. test/jdk/jdk/jfr/jvm/TestLongStringsInPool.java line 64:
62: firstRec.start(); 63: // commit events with empty message (both recordings 64: // will have the same number of events
Missing ")". ------------- PR Review: https://git.openjdk.org/jdk/pull/20596#pullrequestreview-2254271311 PR Review Comment: https://git.openjdk.org/jdk/pull/20596#discussion_r1726889099 PR Review Comment: https://git.openjdk.org/jdk/pull/20596#discussion_r1726889453
When committing JFR events with string data longer than 128 characters, the string isn't added to the JFR string pool. In scenarios with many events containing large string values, this can lead to very large JFR recordings. This is mostly the case for custom events containing f.i. SQL strings or other data. In the case where the string data consist of mostly duplicate data, adding them to the string pool has been shown to reduce the recording size by a factor of 10.
This change will add all longer strings (>128 characters) to the string pool. Strings of length 16-128 are still added using the same pre-cache methodology as before. The raised StringPool.MAX_LIMIT is used by [`jdk.jfr.internal.event.EventWriter::putString`](https://github.com/openjdk/jdk/blob/master/src/jdk.jfr/share/classes/jdk/jfr...) to determine which strings to add to the pool.
## Testing - tier1-tier3 - jdk_jfr - Internal testing with high load, using custom JFR events containing long strings have been performed
Joakim Nordström has updated the pull request incrementally with one additional commit since the last revision: Fixed test ------------- Changes: - all: https://git.openjdk.org/jdk/pull/20596/files - new: https://git.openjdk.org/jdk/pull/20596/files/a55f52a2..1c44df36 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20596&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20596&range=00-01 Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/20596.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20596/head:pull/20596 PR: https://git.openjdk.org/jdk/pull/20596
On Thu, 22 Aug 2024 11:51:45 GMT, Joakim Nordström <jnordstrom@openjdk.org> wrote:
When committing JFR events with string data longer than 128 characters, the string isn't added to the JFR string pool. In scenarios with many events containing large string values, this can lead to very large JFR recordings. This is mostly the case for custom events containing f.i. SQL strings or other data. In the case where the string data consist of mostly duplicate data, adding them to the string pool has been shown to reduce the recording size by a factor of 10.
This change will add all longer strings (>128 characters) to the string pool. Strings of length 16-128 are still added using the same pre-cache methodology as before. The raised StringPool.MAX_LIMIT is used by [`jdk.jfr.internal.event.EventWriter::putString`](https://github.com/openjdk/jdk/blob/master/src/jdk.jfr/share/classes/jdk/jfr...) to determine which strings to add to the pool.
## Testing - tier1-tier3 - jdk_jfr - Internal testing with high load, using custom JFR events containing long strings have been performed
Joakim Nordström has updated the pull request incrementally with one additional commit since the last revision:
Fixed test
test/jdk/jdk/jfr/jvm/TestLongStringsInPool.java line 96:
94: // the files aren't exactly the same size, but rec2 should 95: // not take up space for all strings if they're pooled correctly 96: long maxAllowedDiff = (numEvents - 1) * strLen;
Suggestion: long maxAllowedDiff = (numEvents - 1) * strLen; ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20596#discussion_r1727451114
When committing JFR events with string data longer than 128 characters, the string isn't added to the JFR string pool. In scenarios with many events containing large string values, this can lead to very large JFR recordings. This is mostly the case for custom events containing f.i. SQL strings or other data. In the case where the string data consist of mostly duplicate data, adding them to the string pool has been shown to reduce the recording size by a factor of 10.
This change will add all longer strings (>128 characters) to the string pool. Strings of length 16-128 are still added using the same pre-cache methodology as before. The raised StringPool.MAX_LIMIT is used by [`jdk.jfr.internal.event.EventWriter::putString`](https://github.com/openjdk/jdk/blob/master/src/jdk.jfr/share/classes/jdk/jfr...) to determine which strings to add to the pool.
## Testing - tier1-tier3 - jdk_jfr - Internal testing with high load, using custom JFR events containing long strings have been performed
Joakim Nordström has updated the pull request incrementally with one additional commit since the last revision: Fixed test ------------- Changes: - all: https://git.openjdk.org/jdk/pull/20596/files - new: https://git.openjdk.org/jdk/pull/20596/files/1c44df36..190704fd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20596&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20596&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/20596.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20596/head:pull/20596 PR: https://git.openjdk.org/jdk/pull/20596
On Fri, 23 Aug 2024 07:25:43 GMT, Joakim Nordström <jnordstrom@openjdk.org> wrote:
When committing JFR events with string data longer than 128 characters, the string isn't added to the JFR string pool. In scenarios with many events containing large string values, this can lead to very large JFR recordings. This is mostly the case for custom events containing f.i. SQL strings or other data. In the case where the string data consist of mostly duplicate data, adding them to the string pool has been shown to reduce the recording size by a factor of 10.
This change will add all longer strings (>128 characters) to the string pool. Strings of length 16-128 are still added using the same pre-cache methodology as before. The raised StringPool.MAX_LIMIT is used by [`jdk.jfr.internal.event.EventWriter::putString`](https://github.com/openjdk/jdk/blob/master/src/jdk.jfr/share/classes/jdk/jfr...) to determine which strings to add to the pool.
## Testing - tier1-tier3 - jdk_jfr - Internal testing with high load, using custom JFR events containing long strings have been performed
Joakim Nordström has updated the pull request incrementally with one additional commit since the last revision:
Fixed test
I've fixed the nits for the test, thanks @turbanoff and @mgronlun! This now needs a re-review. Thanks! ------------- PR Comment: https://git.openjdk.org/jdk/pull/20596#issuecomment-2306624089
On Fri, 23 Aug 2024 07:25:43 GMT, Joakim Nordström <jnordstrom@openjdk.org> wrote:
When committing JFR events with string data longer than 128 characters, the string isn't added to the JFR string pool. In scenarios with many events containing large string values, this can lead to very large JFR recordings. This is mostly the case for custom events containing f.i. SQL strings or other data. In the case where the string data consist of mostly duplicate data, adding them to the string pool has been shown to reduce the recording size by a factor of 10.
This change will add all longer strings (>128 characters) to the string pool. Strings of length 16-128 are still added using the same pre-cache methodology as before. The raised StringPool.MAX_LIMIT is used by [`jdk.jfr.internal.event.EventWriter::putString`](https://github.com/openjdk/jdk/blob/master/src/jdk.jfr/share/classes/jdk/jfr...) to determine which strings to add to the pool.
## Testing - tier1-tier3 - jdk_jfr - Internal testing with high load, using custom JFR events containing long strings have been performed
Joakim Nordström has updated the pull request incrementally with one additional commit since the last revision:
Fixed test
Marked as reviewed by mgronlun (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/20596#pullrequestreview-2256786206
On Fri, 23 Aug 2024 09:24:57 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:
Joakim Nordström has updated the pull request incrementally with one additional commit since the last revision:
Fixed test
Marked as reviewed by mgronlun (Reviewer).
Thank you @mgronlun! Could I also request a sponsor for this? Thanks! ------------- PR Comment: https://git.openjdk.org/jdk/pull/20596#issuecomment-2306683553
On Fri, 23 Aug 2024 07:25:43 GMT, Joakim Nordström <jnordstrom@openjdk.org> wrote:
When committing JFR events with string data longer than 128 characters, the string isn't added to the JFR string pool. In scenarios with many events containing large string values, this can lead to very large JFR recordings. This is mostly the case for custom events containing f.i. SQL strings or other data. In the case where the string data consist of mostly duplicate data, adding them to the string pool has been shown to reduce the recording size by a factor of 10.
This change will add all longer strings (>128 characters) to the string pool. Strings of length 16-128 are still added using the same pre-cache methodology as before. The raised StringPool.MAX_LIMIT is used by [`jdk.jfr.internal.event.EventWriter::putString`](https://github.com/openjdk/jdk/blob/master/src/jdk.jfr/share/classes/jdk/jfr...) to determine which strings to add to the pool.
## Testing - tier1-tier3 - jdk_jfr - Internal testing with high load, using custom JFR events containing long strings have been performed
Joakim Nordström has updated the pull request incrementally with one additional commit since the last revision:
Fixed test
@jaokim Your change (at version 190704fd702567a75be32129cd64ddd23d2b9a07) is now ready to be sponsored by a Committer. ------------- PR Comment: https://git.openjdk.org/jdk/pull/20596#issuecomment-2306685219
On Thu, 15 Aug 2024 13:16:24 GMT, Joakim Nordström <jnordstrom@openjdk.org> wrote:
When committing JFR events with string data longer than 128 characters, the string isn't added to the JFR string pool. In scenarios with many events containing large string values, this can lead to very large JFR recordings. This is mostly the case for custom events containing f.i. SQL strings or other data. In the case where the string data consist of mostly duplicate data, adding them to the string pool has been shown to reduce the recording size by a factor of 10.
This change will add all longer strings (>128 characters) to the string pool. Strings of length 16-128 are still added using the same pre-cache methodology as before. The raised StringPool.MAX_LIMIT is used by [`jdk.jfr.internal.event.EventWriter::putString`](https://github.com/openjdk/jdk/blob/master/src/jdk.jfr/share/classes/jdk/jfr...) to determine which strings to add to the pool.
## Testing - tier1-tier3 - jdk_jfr - Internal testing with high load, using custom JFR events containing long strings have been performed
This pull request has now been integrated. Changeset: d5c6158c Author: Joakim Nordström <jnordstrom@openjdk.org> Committer: Markus Grönlund <mgronlun@openjdk.org> URL: https://git.openjdk.org/jdk/commit/d5c6158cedfd96a9f97d83355b10730b81274648 Stats: 119 lines in 2 files changed: 116 ins; 0 del; 3 mod 8338389: [JFR] Long strings should be added to the string pool Reviewed-by: mgronlun ------------- PR: https://git.openjdk.org/jdk/pull/20596
participants (4)
-
Andrey Turbanov
-
duke
-
Joakim Nordström
-
Markus Grönlund