changeset 59175:068a5c842bb4

8244463: JFR: Clean up jdk.jfr.internal.RepositoryChunk Reviewed-by: jbachorik, mgronlun
author egahlin
date Wed, 06 May 2020 13:31:00 +0200
parents f5373fc53bed
children 1df38fc7d842
files src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecorder.java src/jdk.jfr/share/classes/jdk/jfr/internal/Repository.java src/jdk.jfr/share/classes/jdk/jfr/internal/RepositoryChunk.java src/jdk.jfr/share/classes/jdk/jfr/internal/Utils.java
diffstat 4 files changed, 60 insertions(+), 56 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecorder.java	Mon Apr 27 11:00:29 2020 +0200
+++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecorder.java	Wed May 06 13:31:00 2020 +0200
@@ -36,6 +36,7 @@
 import java.security.AccessController;
 import java.time.Duration;
 import java.time.Instant;
+import java.time.ZonedDateTime;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -219,7 +220,8 @@
 
     synchronized long start(PlatformRecording recording) {
         // State can only be NEW or DELAYED because of previous checks
-        Instant now = Instant.now();
+        ZonedDateTime zdtNow = ZonedDateTime.now();
+        Instant now = zdtNow.toInstant();
         recording.setStartTime(now);
         recording.updateTimer();
         Duration duration = recording.getDuration();
@@ -242,8 +244,8 @@
         if (beginPhysical) {
             RepositoryChunk newChunk = null;
             if (toDisk) {
-                newChunk = repository.newChunk(now);
-                MetadataRepository.getInstance().setOutput(newChunk.getUnfinishedFile().toString());
+                newChunk = repository.newChunk(zdtNow);
+                MetadataRepository.getInstance().setOutput(newChunk.getFile().toString());
             } else {
                 MetadataRepository.getInstance().setOutput(null);
             }
@@ -256,9 +258,9 @@
         } else {
             RepositoryChunk newChunk = null;
             if (toDisk) {
-                newChunk = repository.newChunk(now);
+                newChunk = repository.newChunk(zdtNow);
                 RequestEngine.doChunkEnd();
-                MetadataRepository.getInstance().setOutput(newChunk.getUnfinishedFile().toString());
+                MetadataRepository.getInstance().setOutput(newChunk.getFile().toString());
                 startNanos = jvm.getChunkStartNanos();
             }
             recording.setState(RecordingState.RUNNING);
@@ -286,7 +288,8 @@
         if (Utils.isBefore(state, RecordingState.RUNNING)) {
             throw new IllegalStateException("Recording must be started before it can be stopped.");
         }
-        Instant now = Instant.now();
+        ZonedDateTime zdtNow = ZonedDateTime.now();
+        Instant now = zdtNow.toInstant();
         boolean toDisk = false;
         boolean endPhysical = true;
         long streamInterval = Long.MAX_VALUE;
@@ -325,8 +328,8 @@
             RequestEngine.doChunkEnd();
             updateSettingsButIgnoreRecording(recording);
             if (toDisk) {
-                newChunk = repository.newChunk(now);
-                MetadataRepository.getInstance().setOutput(newChunk.getUnfinishedFile().toString());
+                newChunk = repository.newChunk(zdtNow);
+                MetadataRepository.getInstance().setOutput(newChunk.getFile().toString());
             } else {
                 MetadataRepository.getInstance().setOutput(null);
             }
@@ -375,13 +378,13 @@
 
 
     synchronized void rotateDisk() {
-        Instant now = Instant.now();
+        ZonedDateTime now = ZonedDateTime.now();
         RepositoryChunk newChunk = repository.newChunk(now);
         RequestEngine.doChunkEnd();
-        MetadataRepository.getInstance().setOutput(newChunk.getUnfinishedFile().toString());
+        MetadataRepository.getInstance().setOutput(newChunk.getFile().toString());
         writeMetaEvents();
         if (currentChunk != null) {
-            finishChunk(currentChunk, now, null);
+            finishChunk(currentChunk, now.toInstant(), null);
         }
         currentChunk = newChunk;
         RequestEngine.doChunkBegin();
--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/Repository.java	Mon Apr 27 11:00:29 2020 +0200
+++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/Repository.java	Wed May 06 13:31:00 2020 +0200
@@ -27,9 +27,8 @@
 
 import java.io.IOException;
 import java.nio.file.Path;
-import java.time.Instant;
 import java.time.LocalDateTime;
-import java.time.format.DateTimeFormatter;
+import java.time.ZonedDateTime;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -41,8 +40,6 @@
     private static final JVM jvm = JVM.getJVM();
     private static final Repository instance = new Repository();
 
-    public final static DateTimeFormatter REPO_DATE_FORMAT = DateTimeFormatter
-            .ofPattern("yyyy_MM_dd_HH_mm_ss");
     private static final String JFR_REPOSITORY_LOCATION_PROPERTY = "jdk.jfr.repository";
 
     private final Set<SafePath> cleanupDirectories = new HashSet<>();
@@ -80,7 +77,7 @@
         }
     }
 
-    synchronized RepositoryChunk newChunk(Instant timestamp) {
+    synchronized RepositoryChunk newChunk(ZonedDateTime timestamp) {
         try {
             if (!SecuritySupport.existDirectory(repository)) {
                 this.repository = createRepository(baseLocation);
@@ -101,7 +98,7 @@
         SafePath canonicalBaseRepositoryPath = createRealBasePath(basePath);
         SafePath f = null;
 
-        String basename = REPO_DATE_FORMAT.format(LocalDateTime.now()) + "_" + JVM.getJVM().getPid();
+        String basename = Utils.formatDateTime(LocalDateTime.now()) + "_" + JVM.getJVM().getPid();
         String name = basename;
 
         int i = 0;
--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/RepositoryChunk.java	Mon Apr 27 11:00:29 2020 +0200
+++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/RepositoryChunk.java	Wed May 06 13:31:00 2020 +0200
@@ -33,12 +33,12 @@
 import java.time.LocalDateTime;
 import java.time.ZonedDateTime;
 import java.util.Comparator;
-import java.util.Objects;
 
 import jdk.jfr.internal.SecuritySupport.SafePath;
 
 final class RepositoryChunk {
     private static final int MAX_CHUNK_NAMES = 100;
+    private static final String FILE_EXTENSION = ".jfr";
 
     static final Comparator<RepositoryChunk> END_TIME_COMPARATOR = new Comparator<RepositoryChunk>() {
         @Override
@@ -48,8 +48,7 @@
     };
 
     private final SafePath repositoryPath;
-    private final SafePath unFinishedFile;
-    private final SafePath file;
+    private final SafePath chunkFile;
     private final Instant startTime;
     private final RandomAccessFile unFinishedRAF;
 
@@ -57,36 +56,28 @@
     private int refCount = 0;
     private long size;
 
-    RepositoryChunk(SafePath path, Instant startTime) throws Exception {
-        ZonedDateTime z = ZonedDateTime.now();
-        String fileName = Repository.REPO_DATE_FORMAT.format(
-                LocalDateTime.ofInstant(startTime, z.getZone()));
-        this.startTime = startTime;
+    RepositoryChunk(SafePath path, ZonedDateTime timestamp) throws Exception {
+        this.startTime = timestamp.toInstant();
         this.repositoryPath = path;
-        this.unFinishedFile = findFileName(repositoryPath, fileName, ".jfr");
-        this.file = findFileName(repositoryPath, fileName, ".jfr");
-        this.unFinishedRAF = SecuritySupport.createRandomAccessFile(unFinishedFile);
- //       SecuritySupport.touch(file);
+        this.chunkFile = findFileName(repositoryPath, timestamp.toLocalDateTime());
+        this.unFinishedRAF = SecuritySupport.createRandomAccessFile(chunkFile);
     }
 
-    private static SafePath findFileName(SafePath directory, String name, String extension) throws Exception {
-        Path p = directory.toPath().resolve(name + extension);
+    private static SafePath findFileName(SafePath directory, LocalDateTime time) throws Exception {
+        String filename = Utils.formatDateTime(time);
+        Path p = directory.toPath().resolve(filename + FILE_EXTENSION);
         for (int i = 1; i < MAX_CHUNK_NAMES; i++) {
             SafePath s = new SafePath(p);
             if (!SecuritySupport.exists(s)) {
                 return s;
             }
-            String extendedName = String.format("%s_%02d%s", name, i, extension);
+            String extendedName = String.format("%s_%02d%s", filename, i, FILE_EXTENSION);
             p = directory.toPath().resolve(extendedName);
         }
-        p = directory.toPath().resolve(name + "_" + System.currentTimeMillis() + extension);
+        p = directory.toPath().resolve(filename + "_" + System.currentTimeMillis() + FILE_EXTENSION);
         return SecuritySupport.toRealPath(new SafePath(p));
     }
 
-    public SafePath getUnfinishedFile() {
-        return unFinishedFile;
-    }
-
     void finish(Instant endTime) {
         try {
             finishWithException(endTime);
@@ -97,15 +88,9 @@
 
     private void finishWithException(Instant endTime) throws IOException {
         unFinishedRAF.close();
-        this.size = finish(unFinishedFile, file);
+        this.size = SecuritySupport.getFileSize(chunkFile);
         this.endTime = endTime;
-        Logger.log(LogTag.JFR_SYSTEM, LogLevel.DEBUG, () -> "Chunk finished: " + file);
-    }
-
-    private static long finish(SafePath unFinishedFile, SafePath file) throws IOException {
-        Objects.requireNonNull(unFinishedFile);
-        Objects.requireNonNull(file);
-        return SecuritySupport.getFileSize(file);
+        Logger.log(LogTag.JFR_SYSTEM, LogLevel.DEBUG, () -> "Chunk finished: " + chunkFile);
     }
 
     public Instant getStartTime() {
@@ -134,13 +119,11 @@
         if (!isFinished()) {
             finish(Instant.MIN);
         }
-        if (file != null) {
-            delete(file);
-        }
+         delete(chunkFile);
         try {
             unFinishedRAF.close();
         } catch (IOException e) {
-            Logger.log(LogTag.JFR, LogLevel.ERROR, () -> "Could not close random access file: " + unFinishedFile.toString() + ". File will not be deleted due to: " + e.getMessage());
+            Logger.log(LogTag.JFR, LogLevel.ERROR, () -> "Could not close random access file: " + chunkFile.toString() + ". File will not be deleted due to: " + e.getMessage());
         }
     }
 
@@ -181,17 +164,14 @@
 
     @Override
     public String toString() {
-        if (isFinished()) {
-            return file.toString();
-        }
-        return unFinishedFile.toString();
+        return chunkFile.toString();
     }
 
     ReadableByteChannel newChannel() throws IOException {
         if (!isFinished()) {
             throw new IOException("Chunk not finished");
         }
-        return ((SecuritySupport.newFileChannelToRead(file)));
+        return ((SecuritySupport.newFileChannelToRead(chunkFile)));
     }
 
     public boolean inInterval(Instant startTime, Instant endTime) {
@@ -205,6 +185,6 @@
     }
 
     public SafePath getFile() {
-        return file;
+        return chunkFile;
     }
 }
--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/Utils.java	Mon Apr 27 11:00:29 2020 +0200
+++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/Utils.java	Wed May 06 13:31:00 2020 +0200
@@ -180,6 +180,30 @@
         return String.format("%d%s%s", value, separation, result.text);
     }
 
+    // This method reduces the number of loaded classes
+    // compared to DateTimeFormatter
+    static String formatDateTime(LocalDateTime time) {
+        StringBuilder sb = new StringBuilder(19);
+        sb.append(time.getYear() / 100);
+        appendPadded(sb, time.getYear() % 100, true);
+        appendPadded(sb, time.getMonth().getValue(), true);
+        appendPadded(sb, time.getDayOfMonth(), true);
+        appendPadded(sb, time.getHour(), true);
+        appendPadded(sb, time.getMinute(), true);
+        appendPadded(sb, time.getSecond(), false);
+        return sb.toString();
+    }
+
+    private static void appendPadded(StringBuilder text, int number, boolean separator) {
+        if (number < 10) {
+            text.append('0');
+        }
+        text.append(number);
+        if (separator) {
+            text.append('_');
+        }
+    }
+
     public static long parseTimespanWithInfinity(String s) {
         if (INFINITY.equals(s)) {
             return Long.MAX_VALUE;
@@ -604,7 +628,7 @@
 
     public static String makeFilename(Recording recording) {
         String pid = JVM.getJVM().getPid();
-        String date = Repository.REPO_DATE_FORMAT.format(LocalDateTime.now());
+        String date = formatDateTime(LocalDateTime.now());
         String idText = recording == null ? "" :  "-id-" + Long.toString(recording.getId());
         return "hotspot-" + "pid-" + pid + idText + "-" + date + ".jfr";
     }