-
Notifications
You must be signed in to change notification settings - Fork 96
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
--This would allow us to wrap timed phases in a try-with-resources
block:
```java
try(TimedPhase phase = RequestLog.startTiming("timerAllTheThings") {
doAllTheThings();
}
```
--This has several advantages over the current approach:
1. We always want to wrap the code that stops a timer in a finally block.
Otherwise, the timer won't be stopped if we have an
exception. try-with-resources makes that clean.
2. It reduces the chances of typos when starting and stopping
timers. It also reduces the chances of people staring a timer
and then forgetting to stop it.
3. It is more concise.
--This has several limitations:
1. The minimum effort code to implement it is awkward. We'll
need to do a much more significant (and not yet clear) refactor
of RequestLog to handle this cleanly.
2. Timers that cross threads can't be handled this way. Those
still need to be stopped and started manually.
--Consequences that are more mixed:
1. It exposes a lot more of the inner workings of RequestLog.
This could make it easier to customers to perform their own
custom timings, but it also exposes code that is very easy
to use incorrectly if you don't have a strong understanding
of it.- Loading branch information
Showing
2 changed files
with
113 additions
and
56 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
88 changes: 88 additions & 0 deletions
88
fili-core/src/main/java/com/yahoo/bard/webservice/logging/TimedPhase.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| // Copyright 2016 Yahoo Inc. | ||
| // Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. | ||
| package com.yahoo.bard.webservice.logging; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import java.util.concurrent.TimeUnit; | ||
|
|
||
| /** | ||
| * Represents a phase that is timed. | ||
| * TimedPhase is used to associate a Timer located in the registry with the exact duration of such a phase for a | ||
| * specific request. | ||
| */ | ||
| public class TimedPhase implements AutoCloseable { | ||
| private static final Logger LOG = LoggerFactory.getLogger(TimedPhase.class); | ||
|
|
||
| private final String name; | ||
| private final TimeUnit unit; | ||
| private long start; | ||
| private long duration; | ||
|
|
||
| /** | ||
| * Constructor. | ||
| * | ||
| * @param name Name of the phase | ||
| * @param unit The timer's units | ||
| */ | ||
| public TimedPhase(String name, TimeUnit unit) { | ||
| this.name = name; | ||
| this.unit = unit; | ||
| } | ||
|
|
||
| /** | ||
| * Constructor. | ||
| * Times in nanoseconds. | ||
| * | ||
| * @param name Name of the phase | ||
| */ | ||
| public TimedPhase(String name) { | ||
| this(name, TimeUnit.NANOSECONDS); | ||
| } | ||
|
|
||
| /** | ||
| * Start the phase. | ||
| */ | ||
| public void start() { | ||
| if (start != 0) { | ||
| LOG.warn("Tried to start timer that is already running: {}", name); | ||
| return; | ||
| } | ||
| start = System.nanoTime(); | ||
| } | ||
|
|
||
| /** | ||
| * Stop the phase. | ||
| */ | ||
| public void stop() { | ||
| if (start == 0) { | ||
| LOG.warn("Tried to stop timer that has not been started: {}", name); | ||
| return; | ||
| } | ||
| duration += System.nanoTime() - start; | ||
| start = 0; | ||
| } | ||
|
|
||
| public long getDuration() { | ||
| return duration; | ||
| } | ||
|
|
||
| public String getName() { | ||
| return name; | ||
| } | ||
|
|
||
| public TimeUnit getUnit() { | ||
| return unit; | ||
| } | ||
|
|
||
| public boolean isStarted() { | ||
| return start != 0; | ||
| } | ||
|
|
||
| @Override | ||
| public void close() { | ||
| stop(); | ||
| RequestLog.registerTime(this); | ||
| } | ||
| } |