Copilot commented on code in PR #21601:
URL: https://github.com/apache/echarts/pull/21601#discussion_r3177010565
##########
src/processor/dataSample.ts:
##########
@@ -86,14 +102,21 @@ export default function dataSample(seriesType: string):
StageHandler {
const coordSys = seriesModel.coordinateSystem;
const count = data.count();
// Only cartesian2d support down sampling. Disable it when there
is few data.
- if (count > 10 && coordSys.type === 'cartesian2d' && sampling) {
+ if (count > 10 && coordSys.type === 'cartesian2d' && sampling &&
sampling !== 'none') {
const baseAxis = coordSys.getBaseAxis();
const valueAxis = coordSys.getOtherAxis(baseAxis);
const extent = baseAxis.getExtent();
const dpr = api.getDevicePixelRatio();
// Coordinste system has been resized
const size = Math.abs(extent[1] - extent[0]) * (dpr || 1);
- const rate = Math.round(count / size);
+ if (!isFinite(size) || size <= 0) {
+ return;
+ }
+ if (count <= size) {
+ return;
+ }
+ const dataCount = countDataInAxisExtent(data, baseAxis,
data.mapDimension(baseAxis.dim));
+ const rate = Math.round(dataCount / size);
Review Comment:
`data.mapDimension(baseAxis.dim)` can return `null`/`undefined` (per
`SeriesData.mapDimension` docs). In that case `getDimensionIndex` becomes `-1`,
`store.get(-1, i)` returns `NaN`, and `dataCount` becomes `0`, which will
silently disable sampling even when it should run (potentially a large perf
regression). Please guard for missing `baseDim`/`dimIdx < 0` and fall back to a
safe rate calculation (e.g., use `count` as `dataCount`).
##########
src/processor/dataSample.ts:
##########
@@ -72,6 +74,20 @@ const indexSampler = function (frame: ArrayLike<number>) {
return Math.round(frame.length / 2);
};
+function countDataInAxisExtent(data: SeriesData, baseAxis: Axis, baseDim:
string) {
+ let count = 0;
+ const scale = baseAxis.scale;
+ const store = data.getStore();
+ const dimIdx = data.getDimensionIndex(baseDim);
+ for (let i = 0, len = data.count(); i < len; i++) {
+ const value = store.get(dimIdx, i) as number;
+ if (scale.contain(value)) {
+ count++;
+ }
+ }
+ return count;
Review Comment:
`countDataInAxisExtent` adds an extra full pass over `data` (and uses
`store.get(...)` inside the loop, which does bounds checks + rawIndex mapping
each iteration). When sampling actually happens, this makes the processor do
~2x O(n) work per update/zoom (count pass + downsample pass). Consider skipping
the count pass when it’s not needed (e.g., if `count !==
seriesModel.getRawData().count()` then `count` is already filtered, so
`dataCount` can just be `count`), or otherwise reducing per-item overhead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
