Benchmark only compares against mapbox releases

[ghost]

For development, I need to be able to benchmark against maplibre releases (see https://github.com/maplibre/maplibre-gl-js/tree/main/bench for benchmarking mapbox / maplibre). However, as reference, maplibre benchmarks still fetch mapbox releases:

maplibre-gl-js/bench/versions/index.html

Lines 21 to 29 in e5e320f

	fetch('https://api.github.com/repos/mapbox/mapbox-gl-js/releases/latest')
	.then(response => response.json())
	.then(pkg => [pkg['tag_name'], 'main']))
	.then(versions => {
	return versions
	.map(v => `https://s3.amazonaws.com/mapbox-gl-js/${v}/benchmarks.js`)
	.concat('/bench/versions/benchmarks_generated.js')
	.reduce((p, script) => p.then(() => loadScript(script)), Promise.resolve());
	})

We can't simply replace this with maplibre specifics, because maplibe does not have any github releases yet:
https://api.github.com/repos/maplibre/maplibre-gl-js/releases/latest returns 404 (at the time of writing).

Even if we explicitly specify versions, there's also no bucket / hosting for maplibre-gl-js benchmark.js: https://s3.amazonaws.com/maplibre-gl-js/main/benchmarks.js

A minor related annoyance: It also still uses mapbox.com for the CSS of the benchmark results page:

maplibre-gl-js/bench/versions/index.html

Line 9 in e5e320f

<link href='https://www.mapbox.com/base/latest/base.css' rel='stylesheet' />

---

I have a number of potential performance changes and would like to submit them to maplibre, but I won't be able to test their usefulness if there's no good way to benchmark against the main branch, without convoluted hacks.

When porting my mapbox 1.12.0 changes to maplibre, I noticed that there have already been changes to hot code paths in maplibre or mapbox 1.13.0, so having recent benchmark releases is very much required.

[xabbu42]

I'd like to push this forward. As far as I gather we just need the benchmarks_generated.js for each release somewhere. Would it be possible to add this file as an additional asset to the releases on github? Even better if we could do this retroactively for the made releases. The only downside I see is that this asset is little bit confusing for normal users who don't care about benchmarks and development in general.

[HarelM]

Can you elaborate on what's needed exactly? I'd love to help push this forward as well.

[xabbu42]

The command yarn run build-benchmarks generates a file bench/versions/benchmarks_generated.js which I believe is needed to run the benchmarks for that version. Per default the benchmarks page did run for the local version with modifications and the last released version for comparison. For that it needs to get that file for the released version from somewhere. My suggestion is to just add another asset to the releases on github with that file (I don't know too much about github releases though). So is this a good idea and can it be done retroactively?

[HarelM]

Can you paste the content of this file here?
In theory, it doesn't make sense to me to add a generated js file to every release, but I might be missing out something here...
I believe the benchmark intention is to run only against the last official release, so I'll be surprised if there's a need for anything else uploaded to github releases...

[xabbu42]

Thinking about it we probably can't/shouldn't do it retroactively as the generated file will have mapbox specific stuff hardcoded (I'll try to test that). But why doesn't it make sense to add it to the releases going forward? Just to clarify it would of course be not the same file for every release but the one build from that specific release.

The benchmarks historically where run against the local version and the last official release. But you could also compare against older releases per environment variable.

[HarelM]

@xabbu42 let's focus for now on making it work against latest maplibre release. Later on we can see how to improve it. Sounds fair enough?

[xabbu42]

To work against latest maplibre release we need the benchmarks_generated.js for hat release. That is how it currently is. Of course I will work first on fixing all benchmarks to run against the local checkout if you meant that, but imho it does not hurt to already discuss a solution for this issue as we will need that anyway.

Do you have an objection to adding benchmarks_generated.js as a separate asset to the releases and/or a better alternative?

[HarelM]

I'm missing some basic things here - the latest version is compiled and tested locally. the last release can be fetched from CDN and tested too, locally, I'm not sure I understand why there's a need to add another file, and this is what I want to understand.
I have a problem running this on my machine due to node 14 issues, so if you could upload the content of the generated file I might better understand why it's needed...

[xabbu42]

I don't know why it is the way it is, perhaps mapbox wanted the benchmark code together with the releases so they could benchmark older releases after breaking changes? The generated file is a minified module bundle generated from https://github.com/maplibre/maplibre-gl-js/blob/main/bench/rollup_config_benchmarks.js It is no longer human readable and I doubt you'll get much out of it.

[msbarry]

In addition, we should get the benchmarks running in CI on every pull request. I omitted 2 things from the circleci config when setting up the initial CI: render tests, and benchmarks (because I thought they both needed a remote datasource which wasn't set up then, but is now with the maptiler docs key). @HarelM and @wipfli are getting the render tests working in #326, but we should also track getting the performance stats running on every PR. Should we use this issue to track that or should I open a new one?

This was the initial .circleci config that's no longer running now:

  collect-stats:
    <<: *defaults
    steps:
      - attach_workspace:
          at: .
      - run:
          name: Collect performance stats
          command: node bench/gl-stats.js
      - aws-cli/install
      - run:
          name: Upload performance stats
          command: aws s3 cp data.json.gz s3://mapbox-loading-dock/raw/gl_js.perf_metrics_staging/ci/`git show -s --date=short --format=%cd-%h HEAD`.json.gz

Glad you're interested in working on some performance improvements @JannikGM! When I have more time to get back to some work on maplibre I'm also interested in looking into some performance improvements that jumped out while I was writing up the life-of-a-tile docs.

[HarelM]

I think this issue is good for this.
I think #201 is the relevant effort in terms of PR.
We obviously can't use mapbox s3 bucket so I recommended to try and simplify this as a starting point to allow this to run and later on complicated it to allow benchmarking against different versions. In any case, I agree this is important.

[xabbu42]

This is fixed with #391.