Add cache for coveringTiles · Pull Request #105 · maplibre/maplibre-gl-js

ghost · 2021-03-17T14:49:39Z

Another tiny step towards reducing repaint time for #96.

Occasionally, we'd see a long coveringTiles in our profiler, which gets worse as you have more sources, because covering tiles are collected for each source seperately.

As a short-term solution for this (fighting symptoms, not the cause), we decided to add memoization to our fork. While this doesn't seem to have an impact in practice, it still appears to be a measurable difference, so I'm proposing it here.
We didn't deploy this yet, so this has gotten very little testing.

I wish I could provide better benchmarks, but #76 (and even then, the benchmark might not be suitable for animations).
Also I'm currently working on a macOS machine and have a really hard time to get stable measurements.
I opted to do a rather naive measurement across 1000 frames, with my CPU turbo boost disabled.

Following was measured on a production build using a modified animate-a-line sample on fullscreen map (1680x914 on 1680x1050 display) in Chrome; with a tweak to cause camera zoom on a city in Germany, and the source / layer for the line was duplicated 5 times (with only one of them being animated).

Click here to expand the source-code for my test-case.

<!DOCTYPE html>
<html>
<head>
    <title>Mapbox GL JS debug page</title>
    <meta charset='utf-8'>
    <meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=no">
    <link rel='stylesheet' href='http://localhost:9966/dist/maplibre-gl.css' />
    <style>
        body { margin: 0; padding: 0; }
        html, body, #map { height: 100%; }
    </style>
</head>

<body>
<div id='map'></div>

<script src='http://localhost:9966/dist/maplibre-gl.js'></script>
<script src='http://localhost:9966/debug/access_token_generated.js'></script>
<script>

var map = window.map = new maplibregl.Map({
    container: 'map',
    zoom: 0.5,
    center: [0, 0],
    style: 'mapbox://styles/mapbox/streets-v10',
    hash: false
});

// Create a GeoJSON source with an empty lineString.
var geojson = {
    'type': 'FeatureCollection',
    'features': [
        {
            'type': 'Feature',
            'geometry': {
                'type': 'LineString',
                'coordinates': [[0, 0]]
            }
        }
    ]
};

var speedFactor = 30; // number of frames per longitude degree
var animation; // to store and cancel the animation
var startTime = 0;
var progress = 0; // progress = timestamp - startTime
var resetTime = false; // indicator of whether time reset is needed for the animation
var pauseButton = document.getElementById('pause');

map.on('load', function () {

    // Hack to add multiple sources
    for(let i = 0; i < 5; i++) {
      map.addSource('line' + i, {
          'type': 'geojson',
          'data': geojson
      });

      // add the line which will be modified in the animation
      map.addLayer({
          'id': 'line-animation' + i,
          'type': 'line',
          'source': 'line' + i,
          'layout': {
              'line-cap': 'round',
              'line-join': 'round'
          },
          'paint': {
              'line-color': '#ed6498',
              'line-width': 5,
              'line-opacity': 0.8
          }
      });
    }

    startTime = performance.now();

    animateLine();

    // reset startTime and progress once the tab loses or gains focus
    // requestAnimationFrame also pauses on hidden tabs by default
    document.addEventListener('visibilitychange', function () {
        resetTime = true;
    });

    // animated in a circle as a sine wave along the map.
    function animateLine(timestamp) {
        if (resetTime) {
            // resume previous progress
            startTime = performance.now() - progress;
            resetTime = false;
        } else {
            progress = timestamp - startTime;
        }

        // restart if it finishes a loop
        if (progress > speedFactor * 360) {
            startTime = timestamp;
            geojson.features[0].geometry.coordinates = [];
        } else {
            var x = progress / speedFactor;
            // draw a sine wave with some math.
            var y = Math.sin((x * Math.PI) / 90) * 40;
            // append new coordinates to the lineString
            geojson.features[0].geometry.coordinates.push([x, y]);
            // then update the map
            map.getSource('line' + 0).setData(geojson);

          // Hack to zoom camera
          if (!isNaN(x)) {
            const z = 7 + (x / 50.0) % 10
            map.setZoom(z);
            map.setCenter([7, 51]);
          }

        }

        // Request the next frame of the animation.
        animation = requestAnimationFrame(animateLine);
    }
});

</script>
</body>
</html>

Before this PR

coveringTiles took about 684.4749951851554us per frame (1000 frames)
coveringTiles took about 86.68629624938646us per call (7896 calls)
coveringTiles took about 621.6999990283512us per frame (1000 frames)
coveringTiles took about 78.56691508003932us per call (7913 calls)
coveringTiles took about 708.9000004925765us per frame (1000 frames)
coveringTiles took about 89.98476777006556us per call (7878 calls)

After this PR

coveringTiles took about 470.2399995876476us per frame (1000 frames)
coveringTiles took about 59.74336165514517us per call (7871 calls)
coveringTiles took about 387.3749999329448us per frame (1000 frames)
coveringTiles took about 49.05965044743475us per call (7896 calls)
coveringTiles took about 375.5949986516498us per frame (1000 frames)
coveringTiles took about 47.38771115968329us per call (7926 calls)

The above was measured by hooking the coveringTiles function like this:

  const _transform = map.transform;
  const _transform_coveringTiles = _transform.coveringTiles.bind(_transform);
  let totalDuration = 0;
  let frameCount = 0;
  let callCount = 0;
  map.transform.coveringTiles = (options) => {
    const start = performance.now();
    const result = _transform_coveringTiles(options);
    const finish = performance.now();
    const duration = finish - start;
    totalDuration += duration;
    callCount++;
    return result;
  };
  
  // Add an event to print durations
  map.on("render", () => {
    frameCount++;
    if (frameCount >= 1000) {
      console.log(`coveringTiles took about ${totalDuration / frameCount * 1000}us per frame (${frameCount} frames)`);
      console.log(`coveringTiles took about ${totalDuration / callCount * 1000}us per call (${callCount} calls)`);
      totalDuration = 0;
      frameCount = 0;
      callCount = 0;
    }
  });

Note that this way of profiling does not consider GC or other side-effects!

ghost · 2021-03-17T14:52:23Z

+            return cacheEntry.map(it => new OverscaledTileID(it.overscaledZ, it.wrap, it.zoom, it.x, it.y));
+        };
+
+        const optionsKey = JSON.stringify([options, this._renderWorldCopies]);


Probably the ugliest part about this patch:

Using an array to add this._renderWorldCopies into the hash key, to ensure that changes to that will invalidate the cache.

Using JSON.stringify on all arguments to construct a unique hash key.

github-actions · 2021-03-17T14:54:53Z

Bundle size report:

Size Change: +76 B
Total Size Before: 201 kB
Total Size After: 201 kB

Output file	Before	After	Change
maplibre-gl.js	196 kB	196 kB	+76 B
maplibre-gl.css	4.62 kB	4.62 kB	0 B

ℹ️ View Details

Source file	Before	After	Change
src/geo/transform.js	3.7 kB	3.79 kB	+89 B

ghost · 2021-09-03T15:20:11Z

I was asked to comment on the state of my old PRs.

I think this PR should be postponed until #76 has been solved.
The performance benefit might be negligible, so having more concrete data would be required.

Just accepting this would unnecessarily complicate the codebase and imply that there's a benefit of doing this, when we are unable to confirm this right now.

github-actions · 2021-11-03T01:45:28Z

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions · 2021-11-10T01:45:36Z

This PR was closed because it has been stalled for 7 days with no activity.

ghost · 2021-11-22T10:26:56Z

I don't have time to look into this right now, as the company that I work for no longer prioritizes performance at the moment.
However, I'd be grateful if someone else could rebase and benchmark my changes.

I'll reopen this for exposure.

HarelM · 2021-11-22T10:50:20Z

@xabbu42 @wipfli Any chance you can take a look at this and see if it adds any benefit in terms of performance? I know you guys made the performance test possible...
I would say that this need a new PR instead of trying to fix this one as this was before the typescript change...

wipfli · 2021-11-22T21:52:31Z

I'm happy to mess around with this and learn something from Jannik.

HarelM · 2021-11-29T13:49:08Z

Closing this in favor of #666

ghost commented Mar 17, 2021

View reviewed changes

Add cache for coveringTiles

d5c5f28

wipfli mentioned this pull request Jun 5, 2021

Support custom tiles source #30

Merged

github-actions Bot added the stale label Nov 3, 2021

github-actions Bot closed this Nov 10, 2021

ghost reopened this Nov 22, 2021

github-actions Bot removed the stale label Nov 23, 2021

wipfli referenced this pull request in wipfli/maplibre-gl-js Nov 27, 2021

Copy-paste covering tile cache from #105

037434f

wipfli mentioned this pull request Nov 27, 2021

Add cache for coveringTiles #666

Closed

HarelM closed this Nov 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add cache for coveringTiles#105

Add cache for coveringTiles#105
ghost wants to merge 1 commit intomainfrom
unknown repository

ghost commented Mar 17, 2021 •

edited by ghost

Loading

Uh oh!

ghost Mar 17, 2021

Uh oh!

github-actions Bot commented Mar 17, 2021 •

edited

Loading

Uh oh!

ghost commented Sep 3, 2021

Uh oh!

github-actions Bot commented Nov 3, 2021

Uh oh!

github-actions Bot commented Nov 10, 2021

Uh oh!

ghost commented Nov 22, 2021

Uh oh!

HarelM commented Nov 22, 2021

Uh oh!

wipfli commented Nov 22, 2021

Uh oh!

HarelM commented Nov 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ghost commented Mar 17, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Sep 3, 2021

Uh oh!

github-actions Bot commented Nov 3, 2021

Uh oh!

github-actions Bot commented Nov 10, 2021

Uh oh!

ghost commented Nov 22, 2021

Uh oh!

HarelM commented Nov 22, 2021

Uh oh!

wipfli commented Nov 22, 2021

Uh oh!

HarelM commented Nov 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ghost commented Mar 17, 2021 •

edited by ghost

Loading

github-actions Bot commented Mar 17, 2021 •

edited

Loading