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.

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

[Refactoring]: Easier use of timers #4514

Closed
kwvanderlinde opened this issue Nov 30, 2023 · 0 comments
Closed

[Refactoring]: Easier use of timers #4514

kwvanderlinde opened this issue Nov 30, 2023 · 0 comments
Labels
code-maintenance Adding/editing javadocs, unit tests, formatting.

Comments

@kwvanderlinde
Copy link
Collaborator

Describe the problem

Our CodeTimer give a good way to keep tabs on performance, but they can be a bit unwieldy. Anywhere we want to time something, we have two options:

  1. Create a new CodeTimer. In this case we also need a good place to report the results, and the times will be disconnected from any surrounding context.
  2. Use an existing timer. This is easy if the place we're using it is closely associated with an existing timer or other timed component. Otherwise, we need to plumb a CodeTimer around, which just adds clutter.

Because of that, it can be challenging to time random bits of the codebase which might be an important part of a larger operation, but that is otherwise decoupled.

The improvement you'd like to see

I would like some new static methods on CodeTimer to make timing easier. The goal is that any component can ask for a timer without needing to know what context it is being used in.

The first method creates a new timing context. Usage would be like:

CodeTimer.using("timer name", (timer) -> {
  // Timed code
});

The method would create a new CodeTimer, install it as the current timer for the thread, invoke the callback, and - when the callback completes - generate a report of all timings that took place. This would, e.g., replace the explicit logic in ZoneRenderer.paintComponent(), FogUtil.exposeLastPath(), PackedFile.save(), etc.

The second method would give access to the current timer from anywhere. Usage:

CodeTimer timer = CodeTimer.get();
timer.start("time me");
// ...
timer.stop("time me");

Since this uses whatever timer is current, the times will be included in whatever report is currently being built.

Expected Benefits

It will be easier to time random parts of the code base. Also it will be possible to time the same code in different contexts. And we won't have to duplicate reporting logic everywhere.

Additional Context

This design is inspired by the preview ScopedValue feature and could be naturally implemented on top of it once available.

As a concrete example of the problem today, there isn't a good way right now to time ZoneView. The ZoneView itself doesn't have a good context to associate with a timer, since it has relatively small operations that are used as part of larger actions - rendering, fog exposure, etc. Those larger actions know when to set up and report times (and they already do it today), there's just no good way to wire the ZoneView method up to them. At least not without some terrible coupling, or plumbing a CodeTimer everywhere as a parameter. This is the motivation behind the proposal, that ZoneView should just be able to say "I want to time this thing" and leave it at that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-maintenance Adding/editing javadocs, unit tests, formatting.
Projects
Development

No branches or pull requests

1 participant