8244463: JFR: Clean up jdk.jfr.internal.RepositoryChunk

Reviewed-by: jbachorik, mgronlun
This commit is contained in:
Erik Gahlin 2020-05-06 13:31:00 +02:00
parent a3443d0fd1
commit ca371c9536
4 changed files with 60 additions and 56 deletions

View File

@ -36,6 +36,7 @@ import java.security.AccessControlContext;
import java.security.AccessController; import java.security.AccessController;
import java.time.Duration; import java.time.Duration;
import java.time.Instant; import java.time.Instant;
import java.time.ZonedDateTime;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
@ -219,7 +220,8 @@ public final class PlatformRecorder {
synchronized long start(PlatformRecording recording) { synchronized long start(PlatformRecording recording) {
// State can only be NEW or DELAYED because of previous checks // 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.setStartTime(now);
recording.updateTimer(); recording.updateTimer();
Duration duration = recording.getDuration(); Duration duration = recording.getDuration();
@ -242,8 +244,8 @@ public final class PlatformRecorder {
if (beginPhysical) { if (beginPhysical) {
RepositoryChunk newChunk = null; RepositoryChunk newChunk = null;
if (toDisk) { if (toDisk) {
newChunk = repository.newChunk(now); newChunk = repository.newChunk(zdtNow);
MetadataRepository.getInstance().setOutput(newChunk.getUnfinishedFile().toString()); MetadataRepository.getInstance().setOutput(newChunk.getFile().toString());
} else { } else {
MetadataRepository.getInstance().setOutput(null); MetadataRepository.getInstance().setOutput(null);
} }
@ -256,9 +258,9 @@ public final class PlatformRecorder {
} else { } else {
RepositoryChunk newChunk = null; RepositoryChunk newChunk = null;
if (toDisk) { if (toDisk) {
newChunk = repository.newChunk(now); newChunk = repository.newChunk(zdtNow);
RequestEngine.doChunkEnd(); RequestEngine.doChunkEnd();
MetadataRepository.getInstance().setOutput(newChunk.getUnfinishedFile().toString()); MetadataRepository.getInstance().setOutput(newChunk.getFile().toString());
startNanos = jvm.getChunkStartNanos(); startNanos = jvm.getChunkStartNanos();
} }
recording.setState(RecordingState.RUNNING); recording.setState(RecordingState.RUNNING);
@ -286,7 +288,8 @@ public final class PlatformRecorder {
if (Utils.isBefore(state, RecordingState.RUNNING)) { if (Utils.isBefore(state, RecordingState.RUNNING)) {
throw new IllegalStateException("Recording must be started before it can be stopped."); 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 toDisk = false;
boolean endPhysical = true; boolean endPhysical = true;
long streamInterval = Long.MAX_VALUE; long streamInterval = Long.MAX_VALUE;
@ -325,8 +328,8 @@ public final class PlatformRecorder {
RequestEngine.doChunkEnd(); RequestEngine.doChunkEnd();
updateSettingsButIgnoreRecording(recording); updateSettingsButIgnoreRecording(recording);
if (toDisk) { if (toDisk) {
newChunk = repository.newChunk(now); newChunk = repository.newChunk(zdtNow);
MetadataRepository.getInstance().setOutput(newChunk.getUnfinishedFile().toString()); MetadataRepository.getInstance().setOutput(newChunk.getFile().toString());
} else { } else {
MetadataRepository.getInstance().setOutput(null); MetadataRepository.getInstance().setOutput(null);
} }
@ -375,13 +378,13 @@ public final class PlatformRecorder {
synchronized void rotateDisk() { synchronized void rotateDisk() {
Instant now = Instant.now(); ZonedDateTime now = ZonedDateTime.now();
RepositoryChunk newChunk = repository.newChunk(now); RepositoryChunk newChunk = repository.newChunk(now);
RequestEngine.doChunkEnd(); RequestEngine.doChunkEnd();
MetadataRepository.getInstance().setOutput(newChunk.getUnfinishedFile().toString()); MetadataRepository.getInstance().setOutput(newChunk.getFile().toString());
writeMetaEvents(); writeMetaEvents();
if (currentChunk != null) { if (currentChunk != null) {
finishChunk(currentChunk, now, null); finishChunk(currentChunk, now.toInstant(), null);
} }
currentChunk = newChunk; currentChunk = newChunk;
RequestEngine.doChunkBegin(); RequestEngine.doChunkBegin();

View File

@ -27,9 +27,8 @@ package jdk.jfr.internal;
import java.io.IOException; import java.io.IOException;
import java.nio.file.Path; import java.nio.file.Path;
import java.time.Instant;
import java.time.LocalDateTime; import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter; import java.time.ZonedDateTime;
import java.util.HashSet; import java.util.HashSet;
import java.util.Set; import java.util.Set;
@ -41,8 +40,6 @@ public final class Repository {
private static final JVM jvm = JVM.getJVM(); private static final JVM jvm = JVM.getJVM();
private static final Repository instance = new Repository(); 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 static final String JFR_REPOSITORY_LOCATION_PROPERTY = "jdk.jfr.repository";
private final Set<SafePath> cleanupDirectories = new HashSet<>(); private final Set<SafePath> cleanupDirectories = new HashSet<>();
@ -80,7 +77,7 @@ public final class Repository {
} }
} }
synchronized RepositoryChunk newChunk(Instant timestamp) { synchronized RepositoryChunk newChunk(ZonedDateTime timestamp) {
try { try {
if (!SecuritySupport.existDirectory(repository)) { if (!SecuritySupport.existDirectory(repository)) {
this.repository = createRepository(baseLocation); this.repository = createRepository(baseLocation);
@ -101,7 +98,7 @@ public final class Repository {
SafePath canonicalBaseRepositoryPath = createRealBasePath(basePath); SafePath canonicalBaseRepositoryPath = createRealBasePath(basePath);
SafePath f = null; 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; String name = basename;
int i = 0; int i = 0;

View File

@ -33,12 +33,12 @@ import java.time.Instant;
import java.time.LocalDateTime; import java.time.LocalDateTime;
import java.time.ZonedDateTime; import java.time.ZonedDateTime;
import java.util.Comparator; import java.util.Comparator;
import java.util.Objects;
import jdk.jfr.internal.SecuritySupport.SafePath; import jdk.jfr.internal.SecuritySupport.SafePath;
final class RepositoryChunk { final class RepositoryChunk {
private static final int MAX_CHUNK_NAMES = 100; 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>() { static final Comparator<RepositoryChunk> END_TIME_COMPARATOR = new Comparator<RepositoryChunk>() {
@Override @Override
@ -48,8 +48,7 @@ final class RepositoryChunk {
}; };
private final SafePath repositoryPath; private final SafePath repositoryPath;
private final SafePath unFinishedFile; private final SafePath chunkFile;
private final SafePath file;
private final Instant startTime; private final Instant startTime;
private final RandomAccessFile unFinishedRAF; private final RandomAccessFile unFinishedRAF;
@ -57,36 +56,28 @@ final class RepositoryChunk {
private int refCount = 0; private int refCount = 0;
private long size; private long size;
RepositoryChunk(SafePath path, Instant startTime) throws Exception { RepositoryChunk(SafePath path, ZonedDateTime timestamp) throws Exception {
ZonedDateTime z = ZonedDateTime.now(); this.startTime = timestamp.toInstant();
String fileName = Repository.REPO_DATE_FORMAT.format(
LocalDateTime.ofInstant(startTime, z.getZone()));
this.startTime = startTime;
this.repositoryPath = path; this.repositoryPath = path;
this.unFinishedFile = findFileName(repositoryPath, fileName, ".jfr"); this.chunkFile = findFileName(repositoryPath, timestamp.toLocalDateTime());
this.file = findFileName(repositoryPath, fileName, ".jfr"); this.unFinishedRAF = SecuritySupport.createRandomAccessFile(chunkFile);
this.unFinishedRAF = SecuritySupport.createRandomAccessFile(unFinishedFile);
// SecuritySupport.touch(file);
} }
private static SafePath findFileName(SafePath directory, String name, String extension) throws Exception { private static SafePath findFileName(SafePath directory, LocalDateTime time) throws Exception {
Path p = directory.toPath().resolve(name + extension); String filename = Utils.formatDateTime(time);
Path p = directory.toPath().resolve(filename + FILE_EXTENSION);
for (int i = 1; i < MAX_CHUNK_NAMES; i++) { for (int i = 1; i < MAX_CHUNK_NAMES; i++) {
SafePath s = new SafePath(p); SafePath s = new SafePath(p);
if (!SecuritySupport.exists(s)) { if (!SecuritySupport.exists(s)) {
return 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(extendedName);
} }
p = directory.toPath().resolve(name + "_" + System.currentTimeMillis() + extension); p = directory.toPath().resolve(filename + "_" + System.currentTimeMillis() + FILE_EXTENSION);
return SecuritySupport.toRealPath(new SafePath(p)); return SecuritySupport.toRealPath(new SafePath(p));
} }
public SafePath getUnfinishedFile() {
return unFinishedFile;
}
void finish(Instant endTime) { void finish(Instant endTime) {
try { try {
finishWithException(endTime); finishWithException(endTime);
@ -97,15 +88,9 @@ final class RepositoryChunk {
private void finishWithException(Instant endTime) throws IOException { private void finishWithException(Instant endTime) throws IOException {
unFinishedRAF.close(); unFinishedRAF.close();
this.size = finish(unFinishedFile, file); this.size = SecuritySupport.getFileSize(chunkFile);
this.endTime = endTime; this.endTime = endTime;
Logger.log(LogTag.JFR_SYSTEM, LogLevel.DEBUG, () -> "Chunk finished: " + file); Logger.log(LogTag.JFR_SYSTEM, LogLevel.DEBUG, () -> "Chunk finished: " + chunkFile);
}
private static long finish(SafePath unFinishedFile, SafePath file) throws IOException {
Objects.requireNonNull(unFinishedFile);
Objects.requireNonNull(file);
return SecuritySupport.getFileSize(file);
} }
public Instant getStartTime() { public Instant getStartTime() {
@ -134,13 +119,11 @@ final class RepositoryChunk {
if (!isFinished()) { if (!isFinished()) {
finish(Instant.MIN); finish(Instant.MIN);
} }
if (file != null) { delete(chunkFile);
delete(file);
}
try { try {
unFinishedRAF.close(); unFinishedRAF.close();
} catch (IOException e) { } 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 @@ final class RepositoryChunk {
@Override @Override
public String toString() { public String toString() {
if (isFinished()) { return chunkFile.toString();
return file.toString();
}
return unFinishedFile.toString();
} }
ReadableByteChannel newChannel() throws IOException { ReadableByteChannel newChannel() throws IOException {
if (!isFinished()) { if (!isFinished()) {
throw new IOException("Chunk not finished"); throw new IOException("Chunk not finished");
} }
return ((SecuritySupport.newFileChannelToRead(file))); return ((SecuritySupport.newFileChannelToRead(chunkFile)));
} }
public boolean inInterval(Instant startTime, Instant endTime) { public boolean inInterval(Instant startTime, Instant endTime) {
@ -205,6 +185,6 @@ final class RepositoryChunk {
} }
public SafePath getFile() { public SafePath getFile() {
return file; return chunkFile;
} }
} }

View File

@ -180,6 +180,30 @@ public final class Utils {
return String.format("%d%s%s", value, separation, result.text); 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) { public static long parseTimespanWithInfinity(String s) {
if (INFINITY.equals(s)) { if (INFINITY.equals(s)) {
return Long.MAX_VALUE; return Long.MAX_VALUE;
@ -604,7 +628,7 @@ public final class Utils {
public static String makeFilename(Recording recording) { public static String makeFilename(Recording recording) {
String pid = JVM.getJVM().getPid(); 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()); String idText = recording == null ? "" : "-id-" + Long.toString(recording.getId());
return "hotspot-" + "pid-" + pid + idText + "-" + date + ".jfr"; return "hotspot-" + "pid-" + pid + idText + "-" + date + ".jfr";
} }