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

Better profiling #604

Merged
merged 33 commits into from
Sep 18, 2023
Merged

Better profiling #604

merged 33 commits into from
Sep 18, 2023

Conversation

ennerf
Copy link
Collaborator

@ennerf ennerf commented Aug 20, 2023

This PR builds on top of #603 (dataset styling)

It looks like there were some profiling capabilities in the ProcessingProfiler, but it seems limited to logs and I don't know whether there is any tool to visualize it.

Interfaces

I've been thinking a bit about this, and I think I came up with a nice interface that is extremely flexible while also providing very fine grained access to items of interest. It currently lives in the dataset module, but once it's stable I'd like to turn the base into a separate project so it's usable in non-chart projects as well. It consists of 3 interfaces:

  • a Profiler is something that creates named measurements
interface Profiler {
    DurationMeasure newDuration(String tag);
}
  • a DurationMeasure can measure time by using start and stop. An appropriate clock is chosen internally, but there is also a fallback to record a raw value for cases where timestamps are passed around (e.g. in event sourcing timestamps are often kept with the events)
interface DurationMeasure {
    void start();
    void stop();

   TimeUnit getClockUnit();
   void recordRawValue(long duration);
}
  • Anything that needs to provide profiling information implements the Profileable interface
interface Profileable {
    void setProfiler(Profiler profiler);
}

Each metric of interest gets a dedicated measure. The field initialize to a DISABLED implementation with empty methods that should be able to be removed by dead code elimination.

class SpecialAxis implements Profileable {

    public void layoutChildren() {
        benchLayoutChildren .start();
        // do work...
        benchLayoutChildren.stop();
    }

    public void drawAxis() {
        benchDrawAxis.start();
        // do work...
        benchDrawAxis.stop();
    }

    @Override
    public void setProfiler(Profiler profiler) {
        benchLayoutChildren = profiler.newDuration("specialAxis-layoutChildren");
        benchDrawAxis = profiler.newDuration("specialAxis-drawAxis");
    }

    private DurationMeasure benchLayoutChildren = DurationMeasure.DISABLED;
    private DurationMeasurem benchDrawAxis = DurationMeasure.DISABLED;

}

Examples

Debug printer

The simplest profiler is a debug printer that prints start/stop tags and some timing information to a log.

chart.setProfiler(Profiler.printProfiler(System.out::println, boolean printStartedInfo));
chart-runPreLayout - started
chart-lockDataSets - started
chart-lockDataSets - finished (0.01 ms)
chart-updateAxisRange - started
chart-updateAxisRange - finished (0.39 ms)
chart-runPreLayout - finished (0.53 ms)
chart-cssAndLayout - started
chart-layoutChildren - started
chart-layoutChildren - finished (0.34 ms)
chart-cssAndLayout - finished (0.57 ms)
chart-runPostLayout - started
chart-drawAxes - started
chart-drawAxes - finished (0.09 ms)
chart-drawCanvas - started
chart-drawCanvas - finished (1.16 ms)
chart-runPostLayout - finished (1.36 ms)

Tag selection

Via default methods we can create a fluent API for filtering tags of interest and modify the tags with meta-information for better disambiguation.

var profiler = Profiler.debugPrinter(System.out::println);
chart.getXAxis().setProfiler(profiler.addPrefix("x-").contains("computePrefSize"));
chart.getYAxis().setProfiler(profiler.addPrefix("y-").contains("computePrefSize"));
y-axis-computePrefSize - started
y-axis-computePrefSize - finished (8.85 ms)
x-axis-computePrefSize - started
x-axis-computePrefSize - finished (5.67 ms)
y-axis-computePrefSize - started
y-axis-computePrefSize - finished (0.02 ms)
x-axis-computePrefSize - started
x-axis-computePrefSize - finished (0.01 ms)

HdrHistogram output

HdrHistogram is an established and very low-overhead way to measure production behavior in low-latency environments (banking, real-time systems etc.). It records latencies in high-dynamic range histograms (i.e. resolution depends on the absolute value, so it's very space efficient) and provides a file format that can be read by various tools. I've used it a few times, so I had some code sitting around.

The profiler records histograms and continuously writes them to disk in set intervals. The histograms are stored with a tag so they can be disambiguated again later. Profiling an axis and loading it in HdrHistogramVisualizer (a tool I wrote a long time ago when testing the normal JavaFX Charts) looks as below

chart.getXAxis().setProfiler(Profiler.hdrHistogramProfiler("bench/axis.hlog"));

The top chart shows the maxima for each interval, and the lower chart shows a percentile plot with an exponential x axis.

image

Live chart

Given that this is a charting library, I figured it'd be appropriate to also have a live chart of the real-time generated profiling data:

var profiler = Profiler.chartProfiler("RollingBufferSample");
chart.setProfiler(profiler.matches(".*[run|draw].*"));
chart.getXAxis().setProfiler(profiler.addPrefix("x-").contains("draw"));
chart.getYAxis().setProfiler(profiler.addPrefix("y-").contains("draw"));
beamIntensityRenderer.setProfiler(profiler.addPrefix("beamIntensity-").contains("render"));
dipoleCurrentRenderer.setProfiler(profiler.addPrefix("dipoleCurrent-").contains("render"));

I already had some code for HdrHistogram-like visualizations, so I put together an initial demo of it profiling itself. Here is a video.

image

@pr-explainer-bot
Copy link

Pull Request Review

Hey there! 👋 I've summarized the previous results for you. Here's a markdown document for your pull request review:

Changes

  1. Added a new dependency in the pom.xml file at line 99.
  2. Added new private DurationMeasurement variables in the Chart.java file at lines 105-118.
  3. Added a new method ensureJavaFxPulse() in the Chart.java file at lines 124-137.
  4. Added a new method getCssMetaData() in the Chart.java file at lines 143-148.
  5. Removed the XYChartCss.java file.
  6. Added new imports in the AbstractAxis.java file at lines 6-9.
  7. Implemented the Profileable interface in the AbstractAxis.java file at line 11.
  8. Added new private DurationMeasurement variables in the AbstractAxis.java file at lines 32-35.
  9. Added new method setProfiler() in the AbstractAxis.java file at lines 123-134.
  10. Removed the DataSet parameter from the updateLegend() method in the Legend.java file.

Suggestions

  1. In Chart.java, line 100, change protected final Pane canvasForeground = FXUtils.createUnmanagedPane(); to protected final Pane canvasForeground = StyleUtil.addStyles(new FullSizePane(), "chart-canvas-foreground"); for consistent styling.
  2. In Chart.java, line 164, change protected void invalidated() { to protected void invalidated() { for better code readability.
  3. In Chart.java, line 268, change var canvasArea = StyleUtil.addStyles(new PlotAreaPane(canvas, canvasForeground,...

Bugs

  1. Added a new method updateDataSetIndices() to update the global indices of the datasets.
  2. Modified the rendererChanged() method to call updateDataSetIndices() after adding or removing a renderer.
  3. Modified the datasetsChanged() method to call updateDataSetIndices() after setting the global indices.
  4. Modified the updateLegend() method to accept only the list of renderers instead of both datasets and renderers.
  5. Removed the unused forEachDataSet() method.

Improvements

  1. In Chart.java, line 19, instead of commenting 'Update data ranges', it would be better to use a more descriptive comment like 'Update data ranges and trigger layout updates'.
  2. In Chart.java, line 28, instead of commenting 'Make sure the datasets won't be modified', it would be better to use a more descriptive comment like 'Lock the datasets to prevent modification during layout updates'.
  3. In Chart.java, line 36, instead of commenting 'Update data ranges etc. to trigger anything that might need a layout', it would be better to use a more descriptive comment like 'Update data ranges and trigger layout updates for components that depend on them'.
  4. In Chart.java, line 47, instead of commenting 'Update other components', it would be better to use a more descriptive comment like 'Update other components (renderers, plugins) before layout'.
  5. In Chart.java, line 57, instead of commenting 'nothing to do', it would be better to use a more descriptive comment like 'No layout updates required if the state...

Rating

Please provide a rating from 0 to 10 based on the following criteria:

  • Readability
  • Performance
  • Security

Feel free to add a brief explanation for each criterion.

That's it! Good luck with your pull request! 🚀

By the way, have you heard about our premium plan? It's perfect for analyzing big pull requests like this one. Just thought you might be interested! 😉

@ennerf ennerf changed the base branch from main to ennerf/dataset-styling August 20, 2023 11:01
@sonarcloud
Copy link

sonarcloud bot commented Sep 15, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 35 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ennerf ennerf merged commit 1a353c5 into ennerf/dataset-styling Sep 18, 2023
4 of 10 checks passed
@ennerf ennerf deleted the ennerf/profiling branch September 18, 2023 07:38
@wirew0rm wirew0rm mentioned this pull request Sep 18, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant