diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/StringPool.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/StringPool.java index 6513895069e..ecd8d7aad24 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/StringPool.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/StringPool.java @@ -29,8 +29,8 @@ public final class StringPool { public static final int MIN_LIMIT = 16; - public static final int MAX_LIMIT = 128; /* 0 MAX means disabled */ - + public static final int MAX_LIMIT = 131072; /* 0 MAX means disabled */ + private static final int PRECACHE_THRESHOLD = 128; private static final long DO_NOT_POOL = -1; /* max size */ private static final int MAX_SIZE = 32 * 1024; @@ -114,7 +114,7 @@ public static long addString(String s) { if (lsid != null) { return ensureCurrentGeneration(s, lsid); } - if (!preCache(s)) { + if (s.length() <= PRECACHE_THRESHOLD && !preCache(s)) { /* we should not pool this string */ return DO_NOT_POOL; } diff --git a/test/jdk/jdk/jfr/jvm/TestLongStringsInPool.java b/test/jdk/jdk/jfr/jvm/TestLongStringsInPool.java new file mode 100644 index 00000000000..8ce3fd3859f --- /dev/null +++ b/test/jdk/jdk/jfr/jvm/TestLongStringsInPool.java @@ -0,0 +1,116 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.jfr.jvm; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import jdk.jfr.Event; +import jdk.jfr.Recording; +import jdk.jfr.consumer.RecordingFile; +import jdk.test.lib.Asserts; +import jdk.test.lib.jfr.EventNames; + +/** + * @test + * @summary Verify that duplicate longer strings doesn't take up unneccessary space + * @key jfr + * @requires vm.hasJFR + * @library /test/lib + * @run main/othervm jdk.jfr.jvm.TestLongStringsInPool + */ +public class TestLongStringsInPool { + private static class StringEvent extends Event { + String message; + } + + public static void main(String[] args) throws Exception { + // Create two recordings; first has only one large + // string, second has several occurences of the same + // string. With long strings (>128 chars) being pooled, + // the two recording should be roughly the same size. + final int numEvents = 10; + final String longString = generateString(); + final int strLen = longString.length(); + final StringEvent event = new StringEvent(); + event.message = longString; + + Recording firstRec = new Recording(); + firstRec.start(); + // commit events with empty message (both recordings + // will have the same number of events) + for (int i = 0; i < numEvents - 1; i++) { + event.message = ""; + event.commit(); + } + // commit 1 event with a long string + event.message = longString; + event.commit(); + + firstRec.stop(); + Path rec1 = Paths.get(".", "rec1.jfr"); + firstRec.dump(rec1); + firstRec.close(); + + + Recording secondRec = new Recording(); + secondRec.start(); + // commit events with the same long string + for (int i = 0; i < numEvents - 1; i++) { + event.message = longString; + event.commit(); + } + // commit 1 event with a long string + event.message = longString; + event.commit(); + + secondRec.stop(); + Path rec2 = Paths.get(".", "rec2.jfr"); + secondRec.dump(rec2); + secondRec.close(); + + // the files aren't exactly the same size, but rec2 should + // not take up space for all strings if they're pooled correctly + long maxAllowedDiff = (numEvents - 1) * strLen; + long diff = Math.abs(Files.size(rec2) - Files.size(rec1)); + + Asserts.assertTrue(diff <= maxAllowedDiff, "Size difference between recordings is too large: "+ diff +" > " + maxAllowedDiff); + Asserts.assertFalse(RecordingFile.readAllEvents(rec1).isEmpty(), "No events found in recording 1"); + Asserts.assertFalse(RecordingFile.readAllEvents(rec2).isEmpty(), "No events found in recording 2"); + Asserts.assertEquals(RecordingFile.readAllEvents(rec1).size(), RecordingFile.readAllEvents(rec2).size(), "The recordings don't have the same number of events"); + } + + /** + * Generate a string of 256 chars length. + * @return + */ + private static String generateString() { + final StringBuilder builder = new StringBuilder(); + for (int i = 0; i < 32; i++) { + builder.append("abcdefgh"); + } + return builder.toString(); + } +}