joerghoh commented on code in PR #201:
URL:
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/201#discussion_r2309615849
##########
src/main/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathHandler.java:
##########
@@ -746,10 +743,12 @@ private MapEntry createMapEntry(
private Iterator<Resource> queryUnpaged(String query, ResourceResolver
resolver) {
log.debug("start vanity path query: {}", query);
- long queryStart = System.nanoTime();
+ StopWatch sw = StopWatch.createStarted();
final Iterator<Resource> it = resolver.findResources(query,
"JCR-SQL2");
- long queryElapsed = System.nanoTime() - queryStart;
- log.debug("end vanity path query; elapsed {}ms",
TimeUnit.NANOSECONDS.toMillis(queryElapsed));
+ log.debug(
+ "end vanity path query; elapsed {} ({}ms)",
+ sw.getDuration(),
Review Comment:
why do you log both the ```Duration.toString()``` format and also in
miliseconds? If you log both, would prefer to log the instead of the ISO 8061
format
(https://docs.oracle.com/javase/8/docs/api/java/time/Duration.html#toString--)
something more human-friendly.
##########
src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java:
##########
@@ -776,21 +783,46 @@ private boolean drainSpecificQueue(boolean isAlias,
List<Map.Entry<String, Resou
new ChangeContext(type, path, isAlias, !isAlias),
resolverRefreshed, hasReloadedConfig);
}
+ if (count > 0) {
+ log.info(getTimingMessage(message, sw.getDuration(), count));
+ }
+
// do we need to send an event?
return sendEvent;
}
// Drains the resource event queue for aliases
- private void drainAliasQueue() {
- if (drainSpecificQueue(true, resourceChangeQueueForAliases)) {
+ private void drainAliasQueue(String message) {
+ if (drainSpecificQueue(true, message, resourceChangeQueueForAliases)) {
sendChangeEvent();
}
}
// Drains the resource event queue for vanity paths
- private void drainVanityPathQueue() {
- if (drainSpecificQueue(false, resourceChangeQueueForVanityPaths)) {
+ private void drainVanityPathQueue(String message) {
+ if (drainSpecificQueue(false, message,
resourceChangeQueueForVanityPaths)) {
sendChangeEvent();
}
}
+
+ // builds a string based on (optional) message, duration, number of
operations
+ // computes and inserts the number of operations per second
+ static String getTimingMessage(String description, Duration duration, long
operations) {
Review Comment:
optional sounds like you can also provide a ``null`` value, but I am not
sure if the StringBuilder accepts a ``null`` as a parameter in the constructor.
For that reason I would make it mandatory and not-null.
```suggestion
// builds a string based on message, duration, number of operations
// computes and inserts the number of operations per second
static String getTimingMessage(@NotNull String description, Duration
duration, long operations) {
```
##########
src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java:
##########
@@ -776,21 +783,46 @@ private boolean drainSpecificQueue(boolean isAlias,
List<Map.Entry<String, Resou
new ChangeContext(type, path, isAlias, !isAlias),
resolverRefreshed, hasReloadedConfig);
}
+ if (count > 0) {
+ log.info(getTimingMessage(message, sw.getDuration(), count));
+ }
+
// do we need to send an event?
return sendEvent;
}
// Drains the resource event queue for aliases
- private void drainAliasQueue() {
- if (drainSpecificQueue(true, resourceChangeQueueForAliases)) {
+ private void drainAliasQueue(String message) {
+ if (drainSpecificQueue(true, message, resourceChangeQueueForAliases)) {
sendChangeEvent();
}
}
// Drains the resource event queue for vanity paths
- private void drainVanityPathQueue() {
- if (drainSpecificQueue(false, resourceChangeQueueForVanityPaths)) {
+ private void drainVanityPathQueue(String message) {
+ if (drainSpecificQueue(false, message,
resourceChangeQueueForVanityPaths)) {
sendChangeEvent();
}
}
+
+ // builds a string based on (optional) message, duration, number of
operations
+ // computes and inserts the number of operations per second
+ static String getTimingMessage(String description, Duration duration, long
operations) {
+ StringBuilder result = new StringBuilder(description);
+
+ long nanos = duration.toNanos();
+ if (description != null && !description.isEmpty()) {
Review Comment:
and then this null check is redundant and can be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
