[Chore] Performance of Select Mode for MapboxGL/MaplibreGL Adapters
There have been reports (#44) that selection mode in MapLibreGL Adapter could be better.
I don't have any intimidate ideas of how to improve this as most of the performance improvement options were exhausted by #30. One option for users is to increase the dragEventThrottle up from 5 to 10/15. This may help slightly in rendering performance however it may change the UX of dragging for users (worth just experimenting and seeing what feels right).
Hi @JamesLMilner, I had a quick look at this issue, as I was experiencing something similar in code I was writing. I think (if I'm tracing this code correctly) the setData(...) is getting behind because it's kind of a slow operation. So we can end up getting "lag" on slow devices. So essentially the following series of events ends up getting applied, even though you've already decided to update with another value.
A --> B --> C --> D --> END
A -------------------> B -------------------> C -------------------> D -------------------> END
What we want instead is for change B and C to be skipped, and replaced with a single update to D
A --> B --> C --> D --> END
A ---------------------> D -------------------> END (we skipped B and C here)
The code trail in the maplibre-gl-js code
- https://github.com/maplibre/maplibre-gl-js/blob/fbdb32dc439f41c37d1a6dc2d695f10446b72016/src/source/geojson_source.ts#L222-L227
- https://github.com/maplibre/maplibre-gl-js/blob/fbdb32dc439f41c37d1a6dc2d695f10446b72016/src/source/geojson_source.ts#L345
- https://github.com/maplibre/maplibre-gl-js/blob/fbdb32dc439f41c37d1a6dc2d695f10446b72016/src/source/geojson_source.ts#L378
I've had a go at writing a setDataLazy(...) helper, which appears to be helping locally. However I need to test this more and deal with edge cases / error conditions.
private async _setDataLazy(sourceId: string, newData: any) {
const source = this._map.getSource(sourceId) as GeoJSONSource;
if (source) {
const queueItem = this._queue.get(source);
if (queueItem) {
if (queueItem.nextPromise) {
// If we're already waiting just update the data for the next iteration + the count.
console.log("number waiting:", queueItem.nextCount);
this._queue.set(source, {
...queueItem,
nextData: newData,
nextCount: (queueItem.nextCount ?? 0) + 1,
});
await queueItem.nextPromise;
} else {
// Unwrap a promise.
let resolve: (value: any) => void;
const p = new Promise<boolean>((_resolve) => (resolve = _resolve));
this._queue.set(source, {
...queueItem,
nextData: newData,
nextPromise: p,
nextResolve: resolve!,
nextCount: (queueItem.nextCount ?? 0) + 1,
});
await p;
}
} else {
(source as GeoJSONSource).setData(newData);
const p = new Promise<boolean>((resolve) => {
this._map.on("data", (e: any) => {
// Wait for the update to respond from the worker.
if (
e.dataType === "source" &&
e.sourceDataType === "content" &&
e.sourceId === sourceId
) {
resolve(true);
}
});
});
this._queue.set(source, { pendingPromise: p });
try {
await p;
} catch (err: any) {
console.error(err);
}
// Because we're async previously, fetch the data again to see if we have a pending update.
const queueItemAfter = this._queue.get(source);
this._queue.delete(source);
if (queueItemAfter && queueItemAfter.nextResolve) {
await this._setDataLazy(sourceId, queueItemAfter.nextData);
queueItemAfter.nextResolve(true);
}
}
}
}
There is a branch over at https://github.com/JamesLMilner/terra-draw/compare/main...orangemug:terra-draw:fix/set-data-lazy if you want to give it a try.