Skip to content

Commit

Permalink
Remove String.format from logger calls to improve performance (#363)
Browse files Browse the repository at this point in the history 
* Change logger.info and logger.warn statements, fix basicAuthManagerTest

* remove string formatting from most logger.info

* remove string.format from logger.trace statements

* fix logger statement previously missed

* Finish removing all string.format inside logger calls
  • Loading branch information
@Alexjsenn
Alexjsenn authored Dec 21, 2020
1 parent 753c4f0 commit 999edfb
Show file tree
Hide file tree
Showing 19 changed files with 64 additions and 96 deletions.
7 changes: 2 additions & 5 deletions src/main/java/com/redhat/rhjmc/containerjfr/ContainerJfr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,9 @@ public static void main(String[] args) throws Exception {
final Logger logger = Logger.INSTANCE;
final Environment environment = new Environment();

logger.trace(String.format("env: %s", environment.getEnv().toString()));
logger.trace("env: {}", environment.getEnv().toString());

logger.info(
String.format(
"%s started.",
System.getProperty("java.rmi.server.hostname", "container-jfr")));
logger.info("{} started.", System.getProperty("java.rmi.server.hostname", "container-jfr"));

Client client = DaggerContainerJfr_Client.builder().build();

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/redhat/rhjmc/containerjfr/MainModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public static Gson provideGson(Logger logger) {
@Named(RECORDINGS_PATH)
static Path provideSavedRecordingsPath(Logger logger, Environment env) {
String ARCHIVE_PATH = env.getEnv("CONTAINER_JFR_ARCHIVE_PATH", "/flightrecordings");
logger.info(String.format("Local save path for flight recordings set as %s", ARCHIVE_PATH));
logger.info("Local save path for flight recordings set as {}", ARCHIVE_PATH);
return Paths.get(ARCHIVE_PATH);
}
}
2 changes: 1 addition & 1 deletion src/main/java/com/redhat/rhjmc/containerjfr/commands/internal/CommandRegistryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public Output<?> execute(String commandName, String[] args) {
.getName()
.equals(annotation.annotationType().getName()));
if (deprecated) {
logger.warn(String.format("Command \"%s\" is deprecated", commandName));
logger.warn("Command \"{}\" is deprecated", commandName);
}
return c.execute(args);
} catch (Exception e) {
Expand Down
10 changes: 2 additions & 8 deletions src/main/java/com/redhat/rhjmc/containerjfr/messaging/MessagingModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,11 @@ static int provideWebSocketMaxConnections(Environment env, Logger logger) {
MAX_CONNECTIONS_ENV_VAR,
String.valueOf(DEFAULT_MAX_CONNECTIONS)));
if (maxConn > MAX_CONNECTIONS) {
logger.info(
String.format(
"Requested maximum WebSocket connections %d is too large.",
maxConn));
logger.info("Requested maximum WebSocket connections {} is too large.", maxConn);
return MAX_CONNECTIONS;
}
if (maxConn < MIN_CONNECTIONS) {
logger.info(
String.format(
"Requested maximum WebSocket connections %d is too small.",
maxConn));
logger.info("Requested maximum WebSocket connections {} is too small.", maxConn);
return MIN_CONNECTIONS;
}
return maxConn;
Expand Down
16 changes: 6 additions & 10 deletions src/main/java/com/redhat/rhjmc/containerjfr/messaging/MessagingServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public class MessagingServer implements AutoCloseable {
}

public void start() throws SocketException, UnknownHostException {
logger.info(String.format("Max concurrent WebSocket connections: %d", maxConnections));
logger.info("Max concurrent WebSocket connections: {}", maxConnections);

server.websocketHandler(
(sws) -> {
Expand All @@ -107,22 +107,18 @@ public void start() throws SocketException, UnknownHostException {
synchronized (connections) {
if (connections.size() >= maxConnections) {
logger.info(
String.format(
"Dropping remote client %s due to too many concurrent connections",
remoteAddress));
"Dropping remote client {} due to too many concurrent connections",
remoteAddress);
sws.reject();
sendClientActivityNotification(remoteAddress, "dropped");
return;
}
logger.info(String.format("Connected remote client %s", remoteAddress));
logger.info("Connected remote client {}", remoteAddress);

WsClient crw = new WsClient(this.logger, sws);
sws.closeHandler(
(unused) -> {
logger.info(
String.format(
"Disconnected remote client %s",
remoteAddress));
logger.info("Disconnected remote client {}", remoteAddress);
sendClientActivityNotification(remoteAddress, "disconnected");
removeConnection(crw);
});
Expand Down Expand Up @@ -173,7 +169,7 @@ public String readMessage() {

public void writeMessage(WsMessage message) {
String json = gson.toJson(message);
logger.info(String.format("Outgoing WS message: %s", json));
logger.info("Outgoing WS message: {}", json);
synchronized (connections) {
connections.keySet().forEach(c -> c.writeMessage(json));
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/redhat/rhjmc/containerjfr/messaging/WsClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class WsClient implements AutoCloseable, Handler<String> {

@Override
public void handle(String msg) {
logger.info(String.format("(%s): CMD %s", this.sws.remoteAddress().toString(), msg));
logger.info("({}): CMD {}", this.sws.remoteAddress().toString(), msg);
inQ.add(msg);
}

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/redhat/rhjmc/containerjfr/net/BasicAuthManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,15 @@ public Future<Boolean> validateWebSocketSubProtocol(Supplier<String> subProtocol
synchronized void loadConfig() {
Path properties = fs.pathOf(System.getProperty("user.home"), USER_PROPERTIES_FILENAME);
if (!fs.exists(properties)) {
logger.warn(String.format("User properties file \"%s\" does not exist", properties));
logger.warn("User properties file \"{}\" does not exist", properties);
return;
}
if (!fs.isRegularFile(properties)) {
logger.warn(String.format("User properties path \"%s\" is not a file", properties));
logger.warn("User properties path \"{}\" is not a file", properties);
return;
}
if (!fs.isReadable(properties)) {
logger.warn(String.format("User properties file \"%s\" is not readable", properties));
logger.warn("User properties file \"{}\" is not readable", properties);
return;
}
try (Reader br = fs.readFile(properties)) {
Expand Down
11 changes: 5 additions & 6 deletions src/main/java/com/redhat/rhjmc/containerjfr/net/HttpServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,11 @@ public void start() throws SocketException, UnknownHostException {
future.join(); // wait for async deployment to complete

logger.info(
String.format(
"%s service running on %s://%s:%d",
isSsl() ? "HTTPS" : "HTTP",
isSsl() ? "https" : "http",
netConf.getWebServerHost(),
netConf.getExternalWebServerPort()));
"{} service running on {}://{}:{}",
isSsl() ? "HTTPS" : "HTTP",
isSsl() ? "https" : "http",
netConf.getWebServerHost(),
netConf.getExternalWebServerPort());
this.isAlive = true;
}

Expand Down
11 changes: 4 additions & 7 deletions src/main/java/com/redhat/rhjmc/containerjfr/net/SslConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,7 @@ class SslConfiguration {
Path path = obtainKeyStorePathIfSpecified();
if (path != null) {
strategy = new KeyStoreStrategy(path, env.getEnv(KEYSTORE_PASS_ENV, ""));
logger.info(
String.format(
"Selected SSL KeyStore strategy with keystore %s",
path.toString()));
logger.info("Selected SSL KeyStore strategy with keystore {}", path.toString());
return;
}
}
Expand All @@ -91,9 +88,9 @@ class SslConfiguration {
Path cert = pair.getRight();
strategy = new KeyCertStrategy(key, cert);
logger.info(
String.format(
"Selected SSL KeyCert strategy with key %s and cert %s",
key.toString(), cert.toString()));
"Selected SSL KeyCert strategy with key {} and cert {}",
key.toString(),
cert.toString());
return;
}
}
Expand Down
8 changes: 2 additions & 6 deletions src/main/java/com/redhat/rhjmc/containerjfr/net/TargetConnectionManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private JFRConnection attemptConnectAsHostPortPair(ConnectionDescriptor connecti

private JFRConnection connect(JMXServiceURL url, Optional<Credentials> credentials)
throws Exception {
logger.trace(String.format("Locking connection %s", url.toString()));
logger.trace("Locking connection {}", url.toString());
lock.lockInterruptibly();
return jfrConnectionToolkit
.get()
Expand All @@ -141,11 +141,7 @@ private JFRConnection connect(JMXServiceURL url, Optional<Credentials> credentia
credentials.orElse(null),
List.of(
lock::unlock,
() ->
logger.trace(
String.format(
"Unlocking connection %s",
url.toString()))));
() -> logger.trace("Unlocking connection {}", url.toString())));
}

public interface ConnectedTask<T> {
Expand Down
10 changes: 4 additions & 6 deletions src/main/java/com/redhat/rhjmc/containerjfr/net/reports/ActiveRecordingReportCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,10 @@ boolean delete(ConnectionDescriptor connectionDescriptor, String recordingName)
RecordingDescriptor key = new RecordingDescriptor(connectionDescriptor, recordingName);
boolean hasKey = cache.asMap().containsKey(key);
if (hasKey) {
logger.trace(String.format("Invalidated active report cache for %s", recordingName));
logger.trace("Invalidated active report cache for {}", recordingName);
cache.invalidate(key);
} else {
logger.trace(String.format("No cache entry for %s to invalidate", recordingName));
logger.trace("No cache entry for {} to invalidate", recordingName);
}
return hasKey;
}
Expand All @@ -128,9 +128,7 @@ protected String getReport(RecordingDescriptor recordingDescriptor) throws Excep
Path saveFile = null;
try {
generationLock.lock();
logger.trace(
String.format(
"Active report cache miss for %s", recordingDescriptor.recordingName));
logger.trace("Active report cache miss for {}", recordingDescriptor.recordingName);
try {
saveFile =
subprocessReportGeneratorProvider
Expand Down Expand Up @@ -163,7 +161,7 @@ protected String getReport(RecordingDescriptor recordingDescriptor) throws Excep
.findFirst();
if (clone.isPresent()) {
conn.getService().close(clone.get());
logger.trace("Cleaned dangling recording " + cloneName);
logger.trace("Cleaned dangling recording {}", cloneName);
}
return null;
});
Expand Down
7 changes: 2 additions & 5 deletions src/main/java/com/redhat/rhjmc/containerjfr/net/reports/ArchivedRecordingReportCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,7 @@ Future<Path> get(String recordingName) {
.findFirst()
.ifPresentOrElse(
recording -> {
logger.trace(
String.format(
"Archived report cache miss for %s",
recordingName));
logger.trace("Archived report cache miss for {}", recordingName);
try {
Path saveFile =
subprocessReportGeneratorProvider
Expand Down Expand Up @@ -143,7 +140,7 @@ Future<Path> get(String recordingName) {

boolean delete(String recordingName) {
try {
logger.trace(String.format("Invalidating archived report cache for %s", recordingName));
logger.trace("Invalidating archived report cache for {}", recordingName);
return fs.deleteIfExists(getCachedReportPath(recordingName));
} catch (IOException ioe) {
logger.warn(ioe);
Expand Down
7 changes: 3 additions & 4 deletions src/main/java/com/redhat/rhjmc/containerjfr/net/reports/SubprocessReportGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,9 @@ public static void main(String[] args) {
() -> {
long elapsedTime = System.nanoTime() - startTime;
Logger.INSTANCE.info(
String.format(
"%s shutting down after %dms",
SubprocessReportGenerator.class.getName(),
TimeUnit.NANOSECONDS.toMillis(elapsedTime)));
"{} shutting down after {}ms",
SubprocessReportGenerator.class.getName(),
TimeUnit.NANOSECONDS.toMillis(elapsedTime));
}));

var fs = new FileSystem();
Expand Down
26 changes: 12 additions & 14 deletions src/main/java/com/redhat/rhjmc/containerjfr/net/web/WebServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,10 @@ public void start() throws FlightRecorderException, SocketException, UnknownHost
requestHandlers.forEach(
handler -> {
logger.trace(
String.format(
"Registering request handler (priority %d) for [%s]\t%s",
handler.getPriority(), handler.httpMethod(), handler.path()));
"Registering request handler (priority {}) for [{}]\t{}",
handler.getPriority(),
handler.httpMethod(),
handler.path());
Route route;
if (RequestHandler.ALL_PATHS.equals(handler.path())) {
route = router.route();
Expand All @@ -195,9 +196,7 @@ public void start() throws FlightRecorderException, SocketException, UnknownHost
}
route = route.failureHandler(failureHandler);
if (!handler.isAvailable()) {
logger.trace(
String.format(
"%s handler disabled", handler.getClass().getSimpleName()));
logger.trace("{} handler disabled", handler.getClass().getSimpleName());
route = route.disable();
}
});
Expand All @@ -209,14 +208,13 @@ public void start() throws FlightRecorderException, SocketException, UnknownHost
.endHandler(
(res) ->
logger.info(
String.format(
"(%s): %s %s %d %dms",
req.remoteAddress().toString(),
req.method().toString(),
req.path(),
req.response().getStatusCode(),
Duration.between(start, Instant.now())
.toMillis())));
"({}): {} {} {} {}ms",
req.remoteAddress().toString(),
req.method().toString(),
req.path(),
req.response().getStatusCode(),
Duration.between(start, Instant.now())
.toMillis()));
router.handle(req);
});
}
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/com/redhat/rhjmc/containerjfr/net/web/http/api/v1/RecordingsPostHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,7 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
HttpMimeType.JSON.mime())
.end(gson.toJson(Map.of("name", res2.result())));

logger.info(
String.format("Recording saved as %s", res2.result()));
logger.info("Recording saved as {}", res2.result());
}));
}

Expand Down
4 changes: 1 addition & 3 deletions src/main/java/com/redhat/rhjmc/containerjfr/net/web/http/api/v1/TemplatesPostHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,7 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
Path path = fs.pathOf(u.uploadedFileName());
try (InputStream is = fs.newInputStream(path)) {
if (!"template".equals(u.name())) {
logger.info(
String.format(
"Received unexpected file upload named %s", u.name()));
logger.info("Received unexpected file upload named {}", u.name());
continue;
}
templateService.addTemplate(is);
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/com/redhat/rhjmc/containerjfr/net/web/http/generic/HealthGetHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,7 @@ private void checkUri(String envName, String path, CompletableFuture<Boolean> fu
future.complete(false);
return;
}
logger.debug(
String.format("Testing health of %s=%s %s", envName, uri.toString(), path));
logger.debug("Testing health of {}={} {}", envName, uri.toString(), path);
HttpRequest<Buffer> req = webClient.get(uri.getHost(), path);
if (uri.getPort() != -1) {
req = req.port(uri.getPort());
Expand Down
10 changes: 3 additions & 7 deletions src/main/java/com/redhat/rhjmc/containerjfr/platform/PlatformModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,10 @@ static AuthManager provideAuthManager(
final String authManagerClass;
if (env.hasEnv(AUTH_MANAGER_ENV_VAR)) {
authManagerClass = env.getEnv(AUTH_MANAGER_ENV_VAR);
logger.info(String.format("Selecting configured AuthManager \"%s\"", authManagerClass));
logger.info("Selecting configured AuthManager \"{}\"", authManagerClass);
} else {
authManagerClass = platformStrategy.getAuthManager().getClass().getCanonicalName();
logger.info(
String.format(
"Selecting platform default AuthManager \"%s\"", authManagerClass));
logger.info("Selecting platform default AuthManager \"{}\"", authManagerClass);
}
return authManagers.stream()
.filter(mgr -> Objects.equals(mgr.getClass().getCanonicalName(), authManagerClass))
Expand All @@ -109,9 +107,7 @@ static PlatformDetectionStrategy<?> providePlatformStrategy(
PlatformDetectionStrategy<?> strat = null;
if (env.hasEnv(PLATFORM_STRATEGY_ENV_VAR)) {
String platform = env.getEnv(PLATFORM_STRATEGY_ENV_VAR);
logger.info(
String.format(
"Selecting configured PlatformDetectionStrategy \"%s\"", platform));
logger.info("Selecting configured PlatformDetectionStrategy \"{}\"", platform);
for (PlatformDetectionStrategy<?> s : strategies) {
if (Objects.equals(platform, s.getClass().getCanonicalName())) {
strat = s;
Expand Down
Loading

0 comments on commit 999edfb

Please sign in to comment.