Skip to content

fix: JS charts should handle shifts, read subscription data by key #6076

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.

Sign up for GitHub

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

Merged
niloc132 merged 3 commits into from
Sep 19, 2024
Merged

fix: JS charts should handle shifts, read subscription data by key #6076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
61637a2
fix: JS subscriptions data should be read by key, not position
niloc132 Sep 17, 2024
d2719a7
fix: ChartData should handle shifts
niloc132 Sep 17, 2024
bfc0b07
spotless
niloc132 Sep 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions ...api/src/main/java/io/deephaven/web/client/api/subscription/AbstractTableSubscription.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ public Format getFormat(Column column) {
}

/**
* TableData type for both viewports and full table subscriptions.
* TableData type for full table subscriptions.
*/
@TsIgnore
public static class SubscriptionEventData extends UpdateEventData implements ViewportData, SubscriptionTableData {
Expand Down Expand Up @@ -328,6 +328,28 @@ public JsRangeSet getFullIndex() {
}
}

/**
* TableData type for viewport subscriptions.
*/
@TsIgnore
public static class ViewportEventData extends SubscriptionEventData {
public ViewportEventData(WebBarrageSubscription subscription, int rowStyleColumn, JsArray<Column> columns,
RangeSet added, RangeSet removed, RangeSet modified, ShiftedRange[] shifted) {
super(subscription, rowStyleColumn, columns, added, removed, modified, shifted);
}

@Override
public Any getData(long key, Column column) {
return super.getData(fullRowSet.getRange().get(key), column);
}

@Override
public Format getFormat(long index, Column column) {
return super.getFormat(fullRowSet.getRange().get(index), column);
}
}


/**
* Base type to allow trees to extend from here separately from tables.
*/
Expand Down Expand Up @@ -403,7 +425,7 @@ public Any getData(int index, Column column) {

@Override
public Any getData(long key, Column column) {
return subscription.getData(fullRowSet.getRange().get(key), column.getIndex());
return subscription.getData(key, column.getIndex());
}

@Override
Expand All @@ -413,24 +435,23 @@ public Format getFormat(int index, Column column) {

@Override
public Format getFormat(long index, Column column) {
long key = fullRowSet.getRange().get(index);
long cellColors = 0;
long rowColors = 0;
String numberFormat = null;
String formatString = null;
if (column.getStyleColumnIndex() != null) {
LongWrapper wrapper = subscription.getData(key, column.getStyleColumnIndex()).uncheckedCast();
LongWrapper wrapper = subscription.getData(index, column.getStyleColumnIndex()).uncheckedCast();
cellColors = wrapper == null ? 0 : wrapper.getWrapped();
}
if (rowStyleColumn != NO_ROW_FORMAT_COLUMN) {
LongWrapper wrapper = subscription.getData(key, column.getStyleColumnIndex()).uncheckedCast();
LongWrapper wrapper = subscription.getData(index, column.getStyleColumnIndex()).uncheckedCast();
rowColors = wrapper == null ? 0 : wrapper.getWrapped();
}
if (column.getFormatStringColumnIndex() != null) {
numberFormat = subscription.getData(key, column.getFormatStringColumnIndex()).uncheckedCast();
numberFormat = subscription.getData(index, column.getFormatStringColumnIndex()).uncheckedCast();
}
if (column.getFormatStringColumnIndex() != null) {
formatString = subscription.getData(key, column.getFormatStringColumnIndex()).uncheckedCast();
formatString = subscription.getData(index, column.getFormatStringColumnIndex()).uncheckedCast();
}
return new Format(cellColors, rowColors, numberFormat, formatString);
}
Expand Down
2 changes: 1 addition & 1 deletion ...api/src/main/java/io/deephaven/web/client/api/subscription/TableViewportSubscription.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ protected void notifyUpdate(RangeSet rowsAdded, RangeSet rowsRemoved, RangeSet t
if (rowsAdded.size() != rowsRemoved.size() && originalActive) {
fireEventWithDetail(JsTable.EVENT_SIZECHANGED, size());
}
UpdateEventData detail = new SubscriptionEventData(barrageSubscription, rowStyleColumn, getColumns(), rowsAdded,
UpdateEventData detail = new ViewportEventData(barrageSubscription, rowStyleColumn, getColumns(), rowsAdded,
rowsRemoved, totalMods, shifted);

detail.setOffset(this.viewportRowSet.getFirstRow());
Expand Down
182 changes: 51 additions & 131 deletions web/client-api/src/main/java/io/deephaven/web/client/api/widget/plot/ChartData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@

import elemental2.core.JsArray;
import io.deephaven.web.client.api.Column;
import io.deephaven.web.client.api.JsRangeSet;
import io.deephaven.web.client.api.JsTable;
import io.deephaven.web.client.api.TableData;
import io.deephaven.web.client.api.subscription.AbstractTableSubscription;
import io.deephaven.web.client.api.subscription.SubscriptionTableData;
import io.deephaven.web.client.fu.JsSettings;
import io.deephaven.web.shared.data.Range;
import io.deephaven.web.shared.data.RangeSet;
import io.deephaven.web.shared.fu.JsFunction;
import jsinterop.annotations.JsType;
import jsinterop.base.Any;
Expand All @@ -29,118 +28,51 @@
public class ChartData {
private final JsTable table;

private final long[] indexes = new long[0];// in the browser, this array can grow
private RangeSet prevRanges;
private final Map<String, Map<JsFunction<Any, Any>, JsArray<Any>>> cachedData = new HashMap<>();

public ChartData(JsTable table) {
this.table = table;
}

public void update(AbstractTableSubscription.UpdateEventData tableData) {
SubscriptionTableData data = (SubscriptionTableData) tableData;
Iterator<Range> addedIterator = data.getAdded().getRange().rangeIterator();
Iterator<Range> removedIterator = data.getRemoved().getRange().rangeIterator();
Iterator<Range> modifiedIterator = data.getModified().getRange().rangeIterator();

Range nextAdded = addedIterator.hasNext() ? addedIterator.next() : null;
Range nextRemoved = removedIterator.hasNext() ? removedIterator.next() : null;
Range nextModified = modifiedIterator.hasNext() ? modifiedIterator.next() : null;
int i = 0;

// not useful for adding/modifying data, but fast and easy to use when removing rows
JsArray<Any>[] allColumns;
if (nextRemoved != null) {
public void update(AbstractTableSubscription.SubscriptionEventData tableData) {
RangeSet positionsForAddedKeys = tableData.getFullIndex().getRange().invert(tableData.getAdded().getRange());
RangeSet positionsForRemovedKeys = prevRanges.invert(tableData.getRemoved().getRange());
RangeSet positionsForModifiedKeys =
tableData.getFullIndex().getRange().invert(tableData.getModified().getRange());
prevRanges = tableData.getFullIndex().getRange().copy();
// removes first from the previous set of ranges
Iterator<Range> removedPositions = positionsForRemovedKeys.reverseRangeIterator();
JsArray<Any>[] allColumns = null;
if (removedPositions.hasNext()) {
// noinspection unchecked
allColumns = cachedData.values().stream().flatMap(m -> m.values().stream()).toArray(JsArray[]::new);
} else {
allColumns = null;
}

while (nextAdded != null || nextRemoved != null || nextModified != null) {
if (i >= indexes.length) {
// We're past the end, nothing possibly left to remove, just append all the new items
// Note that this is the first case we'll hit on initial snapshot
assert nextRemoved == null;
assert nextModified == null;
while (nextAdded != null) {
insertDataRange(tableData, nextAdded, i);

// increment i past these new items so our offset is correct if there is a next block
i += nextAdded.size();

// not bothering with i or lastIndexSeen since we will break after this while loop
nextAdded = addedIterator.hasNext() ? addedIterator.next() : null;
}
break;
}

long nextIndex = indexes[i];

// check for added items first, since we will insert items just before the current
// index, while the other two start at the current index
if (nextAdded != null && nextAdded.getFirst() < nextIndex) {
assert nextAdded.getLast() < nextIndex;// the whole range should be there if any is

// update the index array and insert the actual data into our mapped columns
insertDataRange(tableData, nextAdded, i);

// increment i past these new items, so that our "next" is actually next
i += nextAdded.size();

nextAdded = addedIterator.hasNext() ? addedIterator.next() : null;
} else if (nextModified != null && nextModified.getFirst() == nextIndex) {
assert nextModified.getLast() >= nextIndex; // somehow being asked to update an item which wasn't
// present

// the updated block is contiguous, make sure there are at least that many items to tweak
assert indexes.length - i >= nextModified.size();

replaceDataRange(tableData, nextModified, i);

// advance i past this section, since no other change can happen to these rows
i += nextModified.size();

nextModified = modifiedIterator.hasNext() ? modifiedIterator.next() : null;
} else if (nextRemoved != null && nextRemoved.getFirst() == nextIndex) {
assert nextRemoved.getLast() >= nextIndex; // somehow being asked to remove an item which wasn't present

// the block being removed is contiguous, so make sure there are at least that many and splice them out
assert indexes.length - i >= nextRemoved.size();

// splice out the indexes
asArray(indexes).splice(i, (int) nextRemoved.size());

// splice out the data itself
assert allColumns != null;
for (JsArray<Any> column : allColumns) {
column.splice(i, (int) nextRemoved.size());
}

// don't in/decrement i, we'll just keep going
nextRemoved = removedIterator.hasNext() ? removedIterator.next() : null;
} else {

// no match, keep reading
i++;
while (removedPositions.hasNext()) {
Range nextRemoved = removedPositions.next();
for (JsArray<Any> column : allColumns) {
column.splice((int) nextRemoved.getFirst(), (int) nextRemoved.size());
}
}

if (JsSettings.isDevMode()) {
assert tableData.getRows().length == indexes.length;
assert cachedData.values().stream().flatMap(m -> m.values().stream())
.allMatch(arr -> arr.length == indexes.length);
assert cachedData.values().stream().flatMap(m -> m.values().stream()).allMatch(arr -> arr
.reduce((Object val, Any p1, int p2) -> ((Integer) val) + 1, 0) == indexes.length);
// then adds
Copy link
Copy Markdown
Contributor

@nbauernfeind nbauernfeind Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this where you should apply shifts?

Copy link
Copy Markdown
Member Author

@niloc132 niloc132 Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(discussed on videocall) This implementation doesn't care about shifts - the rowSet provided by the subscription is already correctly shifted, we just need to splice the values into the (flat) array so that downstream charting can ingest those arrays directly.

Iterator<Range> addedPositions = positionsForAddedKeys.rangeIterator();
while (addedPositions.hasNext()) {
Range nextAdded = addedPositions.next();
insertDataRange(tableData, nextAdded);
}

JsRangeSet fullIndex = ((SubscriptionTableData) tableData).getFullIndex();
PrimitiveIterator.OfLong iter = fullIndex.getRange().indexIterator();
for (int j = 0; j < indexes.length; j++) {
assert indexes[j] == iter.nextLong();
}
Iterator<Range> modifiedPositions = positionsForModifiedKeys.rangeIterator();
while (modifiedPositions.hasNext()) {
Range nextModified = modifiedPositions.next();
replaceDataRange(tableData, nextModified);
}
}

private void replaceDataRange(AbstractTableSubscription.UpdateEventData tableData, Range range, int offset) {
private void replaceDataRange(SubscriptionTableData tableData, Range positions) {
RangeSet keys = tableData.getFullIndex().getRange()
.subsetForPositions(RangeSet.ofRange(positions.getFirst(), positions.getLast()), true);
// we don't touch the indexes at all, only need to walk each column and replace values in this range
for (Entry<String, Map<JsFunction<Any, Any>, JsArray<Any>>> columnMap : cachedData.entrySet()) {
Column col = table.findColumn(columnMap.getKey());
Expand All @@ -149,23 +81,27 @@ private void replaceDataRange(AbstractTableSubscription.UpdateEventData tableDat
JsArray<Any> arr = mapFuncAndArray.getValue();

// rather than getting a slice and splicing it in, just update each value
PrimitiveIterator.OfLong iter = keys.indexIterator();
int i = 0;
if (func == null) {
for (int i = 0; i < range.size(); i++) {
arr.setAt(offset + i, tableData.getData(range.getFirst() + i, col));
while (iter.hasNext()) {
arr.setAt(i++, tableData.getData(iter.next(), col));
}
} else {
for (int i = 0; i < range.size(); i++) {
arr.setAt(offset + i, func.apply(tableData.getData(range.getFirst() + i, col)));
while (iter.hasNext()) {
arr.setAt(i++, tableData.getData(iter.next(), col));
}
}
}
}
}

private void insertDataRange(AbstractTableSubscription.UpdateEventData tableData, Range range, int offset) {
// splice in the new indexes
batchSplice(offset, asArray(indexes), longs(range));

/**
* From the event data, insert a contiguous range of rows to each column.
*/
private void insertDataRange(SubscriptionTableData tableData, Range positions) {
RangeSet keys = tableData.getFullIndex().getRange()
.subsetForPositions(RangeSet.ofRange(positions.getFirst(), positions.getLast()), false);
// splice in data to each column
for (Entry<String, Map<JsFunction<Any, Any>, JsArray<Any>>> columnMap : cachedData.entrySet()) {
String columnName = columnMap.getKey();
Expand All @@ -176,13 +112,13 @@ private void insertDataRange(AbstractTableSubscription.UpdateEventData tableData

// here we create a new array and splice it in, to avoid slowly growing the data array in the case
// of many rows being added
Any[] values = values(tableData, func, col, range);
batchSplice(offset, arr, values);
Any[] values = values(tableData, func, col, keys);
batchSplice((int) positions.getFirst(), arr, values);
}
}
}

private Any[] batchSplice(int offset, JsArray<Any> existingData, Any[] dataToInsert) {
private void batchSplice(int offset, JsArray<Any> existingData, Any[] dataToInsert) {
int lengthToInsert = dataToInsert.length;
JsArray<Any> jsArrayToInsert = Js.uncheckedCast(dataToInsert);

Expand All @@ -193,40 +129,24 @@ private Any[] batchSplice(int offset, JsArray<Any> existingData, Any[] dataToIns
existingData.splice(offset + i, 0,
jsArrayToInsert.slice(i, Math.min(i + batchSize, lengthToInsert)).asArray(new Any[0]));
}

return Js.uncheckedCast(existingData);
}

private Any[] values(AbstractTableSubscription.UpdateEventData tableData, JsFunction<Any, Any> mapFunc, Column col,
Range insertedRange) {
private Any[] values(SubscriptionTableData tableData, JsFunction<Any, Any> mapFunc, Column col,
RangeSet keys) {
JsArray<Any> result = new JsArray<>();

PrimitiveIterator.OfLong iter = keys.indexIterator();
if (mapFunc == null) {
for (int i = 0; i < insertedRange.size(); i++) {
result.push(tableData.getData(insertedRange.getFirst() + i, col));
while (iter.hasNext()) {
result.push(tableData.getData(iter.next(), col));
}
} else {
for (int i = 0; i < insertedRange.size(); i++) {
result.push(mapFunc.apply(tableData.getData(insertedRange.getFirst() + i, col)));
while (iter.hasNext()) {
result.push(mapFunc.apply(tableData.getData(iter.next(), col)));
}
}

return Js.uncheckedCast(result);

}

private static JsArray<Any> asArray(Object obj) {
return Js.uncheckedCast(obj);
}

private static Any[] longs(Range range) {
long[] longs = new long[(int) range.size()];

for (int i = 0; i < longs.length; i++) {
longs[i] = range.getFirst() + i;
}

return Js.uncheckedCast(longs);
}

public JsArray<Any> getColumn(String columnName, JsFunction<Any, Any> mappingFunc, TableData currentUpdate) {
Expand Down
8 changes: 4 additions & 4 deletions web/client-api/src/main/java/io/deephaven/web/client/api/widget/plot/FigureSubscription.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,11 @@ private Promise<TableSubscription> subscribe(final Promise<JsTable> tablePromise
this.currentData = new ChartData(table);
sub.addEventListener(TableSubscription.EVENT_UPDATED, e -> {
// refire with specifics for the columns that we're watching here, after updating data arrays
AbstractTableSubscription.UpdateEventData subscriptionUpdateData =
(AbstractTableSubscription.UpdateEventData) e.detail;
AbstractTableSubscription.SubscriptionEventData subscriptionUpdateData =
(AbstractTableSubscription.SubscriptionEventData) e.detail;
currentData.update(subscriptionUpdateData);

CustomEventInit event = CustomEventInit.create();
CustomEventInit<DataUpdateEvent> event = CustomEventInit.create();
event.setDetail(new DataUpdateEvent(includedSeries.toArray(new JsSeries[0]), currentData,
subscriptionUpdateData));
figure.fireEvent(JsFigure.EVENT_UPDATED, event);
Expand All @@ -271,7 +271,7 @@ private Promise<TableSubscription> subscribe(final Promise<JsTable> tablePromise
firstEventFired = true;

if (downsampleAxisRange != null) {
CustomEventInit successInit = CustomEventInit.create();
CustomEventInit<Object[]> successInit = CustomEventInit.create();
successInit.setDetail(includedSeries.toArray());
figure.fireEvent(JsFigure.EVENT_DOWNSAMPLEFINISHED, successInit);
}
Expand Down
Loading