Fix excessive redraw #12931
Fix excessive redraw #12931
Conversation
thomasneirynck
added
Feature:Visualizations
src/ui/public/visualize/visualize.js
Outdated
| notify.error(e); | ||
| }) | ||
| .then(resp => { | ||
| window._old.push($scope.visData); |
There was a problem hiding this comment.
this seems wrong :) (adding stuff to window object)
src/ui/public/visualize/visualize.js
Outdated
| .then(resp => responseHandler($scope.vis, resp), e => { | ||
| .then(requestHandlerResponse => { | ||
| if ($scope.requestHandlerResponse && $scope.requestHandlerResponse === requestHandlerResponse) { | ||
| //we do not need to call the response handler, since there have been no data changes |
There was a problem hiding this comment.
thats not always the case. response handlers receive vis configuration as well, and they may produce different response based on that.
i don't see a reason for skipping this part, could you elaborate a bit ? all we do is save some resources, which shouldn't be that much anyway ...
if we are to skip it, we should make sure vis object didn't change as well ...
There was a problem hiding this comment.
In practice, this saves a new tabify on each ui-state change which happens all the time when panning/zooming the map. this also avoids reprocessing of the result to geojson on each change.
by skipping this, we can also rely on the status.data===false flag to NOT recreate the leaflet layer, because we know the data hasn't changed. This is an important part to keep the map responsive. If we do rerun the reponseHandler, the data flag becomes true (even though it is the same data).
This doesn't really affect other visualizations because they don't push on the ui-state frequently.
src/ui/public/visualize/visualize.js
Outdated
| notify.error(e); | ||
| }) | ||
| .then(resp => { | ||
| window._old.push($scope.visData); | ||
| window._current = resp; |
There was a problem hiding this comment.
where are this window props used ?
There was a problem hiding this comment.
they're not, sorry for the cruft, forgot to remove them when debugging.
thomasneirynck
force-pushed
the
fix/excessive_redraw
branch
2 times, most recently
from
e859170 to
2c744cc
Compare
small mods remove debug statements ensure we run responsHandle if changes in visConfig
thomasneirynck
force-pushed
the
fix/excessive_redraw
branch
from
7c26621 to
7616de6
Compare
There was a problem hiding this comment.
LGTM, but i wonder:
- tabify takes 2-5ms usually, up to 100ms sometimes with tons of data .... do we really need this ?
- this would only apply to maps visualization ... and even there with the introduction of Filter geohash_grid aggregation to map view box with collar #12806 it won't be required anymore (we will actually want to request every time we pan around right ? )
|
wrt second comment. not entirely, there's a buffer around the edge, so we don't really make a request at each pan and zoom, only when it crosses the buffer. plus, it's optional, so we preserve the old way of requesting as well and don't want to regress there. wrt. first comment. yeah, that's inefficient, but we need for spy panel. should check if we can make this slimmer. |
Closes #12919.
This changes a couple of things for efficiency.