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

[5pt,5pt] overhaul of (DataSet) event system #530

Closed
Tracked by #527
wirew0rm opened this issue Jun 29, 2022 · 5 comments · Fixed by #594
Closed
Tracked by #527

[5pt,5pt] overhaul of (DataSet) event system #530

wirew0rm opened this issue Jun 29, 2022 · 5 comments · Fixed by #594
Assignees

Comments

@wirew0rm
Copy link
Member

wirew0rm commented Jun 29, 2022

Goals:

  • more efficient, thread-safe event processing

    • updates ending up in the javafx thread: merge multiple data updates occurring between 2 frames
    • postprocessing: allow skipping intermediate updates if processing is slower than the update rate.
  • event system should work without depending on the javafx framework

  • allow "ParameterMeasurement" style dataset postprocessing defined by the user at runtime

  • Items to be investigated/implemented

    • either use callback based events (currently implemented, but needs improvements)
      • which threads are callbacks executed on? currently: if there is only 1 -> same thread, otherwise parallel dispatch to worker pool
      • currently callbacks often use FxRun to dispatch to the fx application thread -> no deduplication of redundant tasks
    • or disruptor/ring-buffer based:
      • publish events into ring-buffer of the event listener, event listener polls ring-buffer, filters events and acts accordingly
@wirew0rm wirew0rm mentioned this issue Jun 29, 2022
27 tasks
@wirew0rm wirew0rm added this to the chartfx 11.3 milestone Jun 29, 2022
@wirew0rm wirew0rm changed the title new LMAX Disruptor-based event notification system overhaul of (DataSet) event system Apr 6, 2023
@wirew0rm wirew0rm assigned ennerf and unassigned RalphSteinhagen Jun 20, 2023
@wirew0rm wirew0rm changed the title overhaul of (DataSet) event system [5pt] overhaul of (DataSet) event system Jun 20, 2023
@ennerf
Copy link
Collaborator

ennerf commented Jul 23, 2023

Matching the ideas I've mentioned before, I created an event framework based on bit masks where listeners can subscribe to dirty bits that they are interested in. Similar to JavaFX properties, each element can subscribe to others via change-listeners (called if specified bits change from 0 to 1) or invalidation-listeners (called on every event).

Example for an axis that needs to trigger a layout on a property change

// create an axis state object that knows only about axis events
var state = BitState.initDirty(this, ChartBits.AxisMask);

// changes to the axis padding need to recompute the layout and redraw the canvas
// (the set method has the same signature as a JavaFX listener, but without introducing a dependency)
axisPadding.addListener(state.onPropChange(ChartBits.AxisLayout, ChartBits.AxisCanvas)::set);

// trigger JavaFX layouts
state.addChangeListener(ChartBits.AxisLayout, (src, bits) -> requestLayout());

Example for a chart that subscribes and unsubscribes to an axis:

// create a chart state object that knows about all events
var state = BitState.initDirty(this, ChartBits.AxisMask);

// merge changes coming from a relevant axis
axis.getBitState().addChangeListener(this.state);

// trigger a redraw if any axis needs to be drawn
state.addChangeListener(ChartBits.AxisCanvas, (src, bits) -> drawAxesInNextCycle());

// remove an axis that is no longer part of this chart
axis.getBitState().removeChangeListener(this.state);

Example for skipping the draw step if none of the axes has changed

redrawAxes() {
  if (state.isClean(ChartBits.AxisCanvas) {
    return; // all content is still good
  }
  for (var axis : getAxes) {
    axis.redraw();
  }
}

The state bits get cleared after all elements are drawn. Updates use bitwise operations and batch updates automatically, so the process is very efficient. For example 10 data sets changing 100 values will get merged into 10 dataset events that are then merged into a single chart event.

The merging/batching also simplifies scenarios where datasets are updated from non-JavaFX threads, e.g., with a simple runLater() handoff we get a maximum of one runLater call per non-JavaFX thread per dataset, e.g.,

dataSet.setDirty(ChartBits.Data, CharBits.DataRange);
dataSet.getBitState().addChangeListener(this.state, (src, bits) -> runOnFx(() -> this.state.setDirty(bits));

Depending on the needs we could also have variants with thread-local state and/or smarter locking.

I also added debugging utilities to check what triggered an events, e.g.,

// Print every time something needs the canvas content to be updated
state.addInvalidateListener(ChartBits.AxisCanvas, ChartBits.printerWithStackTrace());

e.g. a redraw call coming from [15] setTickUnit. IntelliJ parses the output as a stack trace and provides links to the lines.

image

At the low-level it's based in int and IntSupplier, so users could also create custom elements with custom events, e.g.,

enum CustomEvents implements IntSupplier {
  SomeBit;
  int getAsInt() {
    return bit;
  }
 final int bit = 1 << (ChartEvents.values().length + ordinal());
}

state.setDirty(CustomEvents.SomeBit);

The ennerf/notify-axes branch contains a fully working version for axis redrawing based on the layout branch

@ennerf
Copy link
Collaborator

ennerf commented Jul 28, 2023

The event system has been tried on the entire chart and works as expected

@ennerf
Copy link
Collaborator

ennerf commented Aug 1, 2023

Between #527 and #579 the following things still need to be looked at.

Minor issues pre merge

  • add min size and pref size for the new layout containers
  • set even/odd offset based on tickmark value (only) when label shifting is active
  • indicator offsets and interactions with measurements
  • fix renderer axes that are not part of the chart

Minor checks

  • check why the previous code only updated tickmarks when there were at least 2 ticks
    • no longer necessary. removed
  • should we support strokes (outline) for tick mark labels? At the moment we only do fillText, but not strokeText
    • added for completeness sake
  • update the default CSS file to match the new hierarchy. Remove insets from the main chart area.
  • do we need to explicitly unregister the default legend visibility listener?
  • do we need to support multi-threaded changes to the state listeners?

Future work

  • The axis tick list uses a boxed List<Double>. My understanding is that this was done to be able to use the number as a key in a HashMap, but this is no longer the case and it can change to use a primitive DoubleArrayList
  • ArrayCache (at least when used from the renderer on the JavaFX thread) does not need all the concurrency primitives. We can just use a single growing double[] that is used as the cache for all renderers.
  • The code for determining the Axis scale/ticks/range should be cleaned up. At the moment there are many different overloads and it is unclear when to use which.

Require further discussion

Concurrency in dependent DataSets

Some datasets, i.e., FragmentedDataSet/DimReductionDataSet/MathDataSet, perform math operations on other datasets. The operations can be short enough to do on the JavaFX thread, but they can also take a long time and should be done concurrently.

Concurrent computations should not hold on to the lock to avoid halting the rendering, so they should do a full copy of the relevant data, e.g.,

  • lock source
  • copy relevant source data
  • unlock source
  • compute derived dataset
  • lock dataset
  • update state
  • unlock dataset

I currently envision 3 different modes. For concurrent processing it may be nice to show a progress indicator on the chart or on the legend symbol.

  • immediate/consistent: computed on the JavaFX thread before layout and always consistent
  • concurrent/lagging: concurrent processing that maintains the previous value and updates whenever the processing is done, e.g., for time series that continuously update and show computed data that is ok to be 1 step behind.
  • concurrent/consistent: concurrent processing that clears the previous value when it performs updates, e.g., for computations that should never show any inconsistencies with the displayed data

Another question is when to trigger the background thread. I can currently think of a few options

  • heuristic: something like a RateLimiter that triggers on the first change and waits until there are no more changes within some timeout, e.g., 20ms. This is the previous behavior.
  • explicit: update on explicit triggers
    • on event: we could send a ChartBits.DataSetRecompute event to trigger updates, but that doesn't integrate well with the dirty state and state clearing
    • interface method: add an interface method, e.g., DependentDataSet::update, that explicitly triggers an update. It can be called by the user once the source data is updated, and additionally in the pre-layout method if users forget or don't want to specify it. The dependent dataset can have its own state and ignore triggers if the source data has not changed.

@ennerf
Copy link
Collaborator

ennerf commented Aug 2, 2023

The new event system aggregates events and deterministically draws all state at once. Besides consistency benefits, this results in significant performance gains as it does not redraw the same content multiple times.

Below are some manually gathered counters for how often individual methods run for certain actions.

  • XYChart::redrawCanvas : AbstractAxis::drawAxis(1), AbstractAxis::drawAxis(2), etc.
Action Before After
CategoryAxisSample - open 6 : 3,4 1 : 1,1
CategoryAxisSample - zoom in 4 : 2,2 1 : 1,1
CategoryAxisSample - zoom out 4 : 2,4 1 : 1,1
ChartIndicatorSample - open 7 : 3,4,4,5 1 : 1,1,1,1
ContourChartSample - open 6 : 2,2 1 : 1,1
ContourChartSample - zoom in 7 : 3,2 1 : 1,1
MultipleAxesSample - open 8 : 3,3,3,4 1 : 1,1,1,1
RotatedAxisLabelSample - open 5 : 2,2,2,2,2,... 1 : 1,1,1,1,1,...

@ennerf
Copy link
Collaborator

ennerf commented Aug 2, 2023

The old event system classes are still in there for backwards compatibility. They can be removed once the measurements and dependent datasets are migrated fully (out of scope for this PR).

@ennerf ennerf changed the title [5pt] overhaul of (DataSet) event system [5pt,5pt] overhaul of (DataSet) event system Aug 7, 2023
@wirew0rm wirew0rm linked a pull request Aug 8, 2023 that will close this issue
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 a pull request may close this issue.

3 participants