Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove String.format from logger calls to improve performance #363

Merged
merged 5 commits into from
Dec 21, 2020
Merged

Remove String.format from logger calls to improve performance #363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
277ae79
Change logger.info and logger.warn statements, fix basicAuthManagerTest
Alexjsenn Dec 17, 2020
e473464
remove string formatting from most logger.info
Alexjsenn Dec 17, 2020
7ed65f8
remove string.format from logger.trace statements
Alexjsenn Dec 17, 2020
b698237
fix logger statement previously missed
Alexjsenn Dec 17, 2020
86f8ef9
Finish removing all string.format inside logger calls
Alexjsenn Dec 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions src/main/java/com/redhat/rhjmc/containerjfr/ContainerJfr.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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