Migrate benchmarks

[xabbu42]

This is a WIP to migrate benchmarks from mapbox infrastructure. Currently I just made the changes necessary so the benchmark pages work again. To finish this PR I need to:

• fix the remaining (now disabled) bechmarks (depends on more complex demotiles for benchmarks and debuging #198)
• add back comparision to released versions (depends on Benchmark only compares against mapbox releases #76)
• replace base.css from mapbox
• confirm your changes do not include backports from Mapbox projects (unless with compliant license)
• add an entry inside this element for inclusion in the maplibre-gl-js changelog: <changelog></changelog>

[github-actions]

Bundle size report:

Size Change: +862 B
Total Size Before: 205 kB
Total Size After: 206 kB

Output file	Before	After	Change
maplibre-gl.js	196 kB	197 kB	+268 B
maplibre-gl.css	8.87 kB	9.46 kB	+594 B

ℹ️ View Details

Source file	Before	After	Change
src/ui/control/geolocate_control.js	2.23 kB	2.3 kB	+69 B
src/ui/control/attribution_control.js	1.28 kB	1.33 kB	+49 B
src/ui/popup.js	1.83 kB	1.87 kB	+32 B
src/ui/control/fullscreen_control.js	852 B	883 B	+31 B
src/ui/control/navigation_control.js	1.69 kB	1.72 kB	+28 B
src/ui/map.js	6.26 kB	6.29 kB	+27 B
src/ui/control/logo_control.js	592 B	609 B	+17 B
src/ui/handler/box_zoom.js	802 B	813 B	+11 B
src/ui/anchor.js	204 B	213 B	+9 B
src/ui/handler/shim/touch_zoom_rotate.js	302 B	311 B	+9 B
src/ui/control/scale_control.js	695 B	704 B	+9 B
src/ui/marker.js	2.82 kB	2.83 kB	+8 B
src/ui/handler/shim/drag_pan.js	233 B	241 B	+8 B
src/util/mapbox.js	3.06 kB	3.06 kB	+1 B

[xabbu42]

I will be away for the next 3 weeks but plan to take this up again after that. If anyone wants to take over in the meantime, you're very welcome.

[HarelM]

This might be fixed as part of #209 as we are revisiting the infrastructure of this repo and rewriting things.
It might be better to combine effort there, I don't know...

[HarelM]

This was not solved as part of #209.
I also think most changes here are relevant probably (some are already part of the accesstoken removal in #298).
Let me know if you are on it or not.
We need to make sure version 2.0 is working as expected and so we need to work on it.

[xabbu42]

I was waiting on the merge of #209 and plan to pick this up again. If somebody beats me to it the better :-)

[HarelM]

Continue on the discussion here: #76 and reading the relevant readme I don't think there's a need to upload any generated file (I might be wrong).
When fixing the tests I noticed that there are readme file scattered around in strategic places for the developers to read.
The readme related to bench marks indicates that you can run the bench mark against any version.
The main problem the way I see it is that we would like to stop using flow and the benchmark code is written in flow which might be a bit hard to convert. There's also a few rollup config files there which takes all the code related to the tests and wraps it up, I'm not sure what exactly it does in terms of code and if it replaces anything to facilitate the tests, but from my perspective, the tests should not be bundled but instead simply ran. having said that, if they run in the browser they probably need to be wrapped. So a deep investigation is needed here :-)
the file you mentioned benchmark_generated.js is the outcome of the rollup files.
Feel free to use the 1.x branch to check how it used to work in the past and how it works now.
If you need any info as to what I did in the typescript branch to fix the rollup, test, etc feel free to ping me here or even in slack.
Good luck! :-)

[HarelM]

Also worth noting that I removed code related to access token in #298 which I hope will be merged to main soon, this will create conflict with this branch. Just giving a heads up...

[xabbu42]

Continue on the discussion here: #76 and reading the relevant readme I don't think there's a need to upload any generated file (I might be wrong).

What do you suggest we do about this code then?

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

Lines 25 to 28 in 04ff47d

	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());

[HarelM]

1. Refactor it so that a person can read it. It's extremely hard to understand it with all the thens... honestly :-)
2. Currently, ignore the compare part - it is used to compare older versions and not the current code to the last released version.

After we solve the "easy" problem of comparing the current version, we can think of a good way to compare older versions. It feels to me like there should be a simpler way to run benchmark test without storing anything besides the released version (maplibre-gl.js of a certain version which is obviously stored already in some CDN places) as it should contain all the needed code to facilitate bench marking, that's at least my gut feeling...
If we fail to find a simpler way to run the benchmarks tests we can find a place to store this generated.js files, storing files publicly is not so hard, I have a few ideas in mind starting from what you said about adding this to each version, to creating a repo just for that, to generating them using git tags etc. but I would prefer to go this route if we can't think of a way without it.
But in order to remove the need to store these files we need to better understand how it all works, which I currently don't fully understand.
If you fully understand what this generated file is doing and why we can't run the compare tests without it please share with me and we'll decide how to proceed.

[xabbu42]

1. Refactor it so that a person can read it. It's extremely hard to understand it with all the thens... honestly :-)

I think its quite clear, perhaps I'm more used to this functional style...
it generates a list of urls to https://s3.amazonaws.com/mapbox-gl-js/ for released versions and appends ''/bench/versions/benchmarks_generated.js' to the list for the current code. It then reduces (https://en.wikipedia.org/wiki/Fold_(higher-order_function)) that list to a Promise for when all those urls are fetched and loaded.

2. Currently, ignore the compare part - it is used to compare older versions and not the current code to the last released version.

I believe you are wrong here. But if you're not I'm looking forward to your solution. About the rest of your answer: as I said I'm only interested to do the minimal effort to get the benchmarks running again. If thats not wanted, then somebody else will have to do the deep dive and refactors you are talking about.

[HarelM]

I'm talking about the has(compare) part above the code you sent. I can't understand where the first condition ends and the second begins.
I prefer async await. A lot more readable...
I've tried to read it now a few times without success.
I'm interested in what happens when the url doesn't have compare in it.
I also opened the mapbox latest release url that is fetched there and I didn't finf any variable named pkg or any array, so I'm sure I'm missing out something trivial.

In theory, we can currently compare against mapbox versions that have this generated.js file in s3, right?
Let's start with that - as you said, a minimal way to run these tests.
If you have a working version using the latest code let's discuss what are our options.
Fair enough?

[xabbu42]

Yeah the formatting of the surrounding code is confusing. There is a very well obfuscated extra closing parentheses on this line:

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

Line 23 in 04ff47d

.then(pkg => [pkg['tag_name'], 'main']))

so the code uses either the given list of versions to compare or fetches the version for the last release from github.

the following code which I referenced above ist then used for both cases (so for urls with and without compare parameter).

[HarelM]

Yup :-) that was the part I talked about refactoring :-) the map...reduce... whatever is ok.
In any case, can you make some of it work? A specific comparison between two versions? Even two mapbox versions?
I would like to know that we can have this running and all what's needed to do is change that code to point to a different location to fetch the generated.js files.
I don't think we're there yet. Let's see that we can generate the code for the latest version, see that we can see results in that page, see that we remove the flow stuff from the code.
The location of the generated files might be problematic, but I think there are bigger problems, from my point of view anyway.
If you have a branch with a working version running on node 14 then I'd love to clone it and see the results on my laptop :-)

[wipfli]

I would like to join this party. What is the status?

[wipfli]

Thanks for staring this!

@xabbu42 is it OK for you if we close this pull request and continue with a branch in this repo? See #391. Then collaborating is a bit easier.

[xabbu42]

Yeah thats better. I'll close this pull request. I was waiting on the typescript refactor to finish and am currently checking that the render tests run through on my machines.

last I checked one regression from this branch was that I could no longer start the benchmark server, which worked here. that will be the first thing to fix if that is still the case. after that we can bring back the commented out benchmarks one-by-one (I think a lot of them just depend on a style containing a layer with a hardcoded id, so it should just be a matter of changing to a suitable public style, have some vector data and using the correct layer id)