Allow RequestScheduler to throttle white-listed servers differently. #6445
Conversation
Since HTTP/2 does not have the same performance characteristics/browser limit that HTTP/1.1 does, throttling to 6 requests per server does not make sense. However, disabling throttling completely is also a bad idea because we may end up making too many requests and throwing some away (for example camera flights loading terrain that is no longer in view). As a compromise, I arrived at 18 being a good request limit. I chose this number because many HTTP/1.1 data providers in Cesium use a/b/c subdomains to work around browser limits, so 3 * 6 = 18. By default, I've added the Cesium ion servers to this list, but we can and should add other known/popular HTTP/2 servers used by Cesium in the future. We'll eventually make this public whenever the rest of the RequestScheduler goes public.
|
@mramato, thanks for the pull request! Maintainers, we have a signed CLA from @mramato, so you can review this at any time.
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
|
BTW, I still think throttling at all is the wrong approach, I'd rather see us not throttle but aggressive cancel requests we no longer need. This is what Google Earth for web and Bing maps (the app) see, to do. |
|
The numbers speak for themselves, this is a huge win. Tests were run using Sentinel-2 and Cesium World Terrain in a 1920px x 1080px canvas using Cesium Widget. throttleRequests: 6 (master)Grand canyon horizon: 3686.38818359375ms Everest: 4579.18115234375ms AGI top-down: 1449.674072265625ms throttleRequests: 18Grand canyon horizon: 1988.40673828125ms Everest: 2335.913330078125ms AGI top-down: 1440.6767578125ms throttleRequests: offGrand canyon horizon: 2114.85107421875ms Everest: 2378.81591796875ms AGI top-down: 1415.2431640625ms While I still think our throttling code as a whole needs to be revisited; this is an obvious win and allows HTTP/2 servers to achieve the same or better performance compared to multi-subdomain HTTP/1.1 servers while still throttling to avoid filling our queue with useless tiles. |
|
Bump. Anyone have any thoughts here? I'd really like to get this into May 1 release, it's all behind the scenes so if we need to change it again for June/July we can |
|
In the above numbers, isn't throttleRequests: off always better than throttleRequests: 18 since it doesn't generate all the canceled requests? Should this whitelabel for turning off throttling? Also, I think everyone already understands this, but I want to reiterate that the longer-term holistic solution includes taking into account the processing time, parallelism, and prioritization of requests once they are received so we don't necessarily want to make a ton of parallel requests, especially when the camera is moving quickly. |
Total load time with throttling off will always be faster for static scenes, whether it's HTTP/1.1 or HTTP/2. However, as you alluded to, throttling off with the camera moving can end up requesting a bunch of tiles that are not used (because the camera have moved before the request resolves). Especially over slow connections. That's the main reason I assume we didn't want to disable it completely. I also think the tile load prioritization also requires tiles be in the request scheduler queue (which doesn't happen if the request is not throttled, but @lilleyse can correct me on this if I'm wrong). Just to avoid confusion, We could also consider disabling throttling completely if the camera is not moving, but that might have side affects I'm not aware of. |
|
First of all - really good numbers, especially terrain. I'm pretty much on board with the changes here since 6 is too low for http2. But too much higher than 18 could hurt user experience. When @austinEng was looking into this he noted:
The idea is that if we disable throttling completely we may see worse results even for a static view. All visible tiles would get requested immediately and it may take longer for the nearest tiles to stream in. As far as cancelling requests aggressively goes, I think this will be a great thing to try out. With the 3D Tiles traversal cleanup it should be possible to know when a tile is loading but was not "touched" this frame, making it a candidate for cancelling. It's a bullet point in #6243 for when I have a chance to look at it.
Yeah that's correct. Unthrottled requests get sent immediately and are not sorted by priority. |
|
OK, so it sounds like both of you are on board, are there any changes requested in order to get this merged? |
|
Yes, OK with me, but didn't look at the code, just whatever @lilleyse thinks. |
|
The code looks good. Maybe just needs a test that checks that 18 requests can go through, with none cancelled, when using one of the white listed servers. |
Also replace all usage of `foo.com` with `test.invalid`, since we should always be using `.invalid` as the tld for anything in tests.
|
@lilleyse this is ready for another look, I will right up a separate issue for the bug I found. |
|
Thanks, looks good, I'll merge once CI passes. |
I'm opening this up for discussion, but if we converge/agree on an approach it would be great to get this simple change merged for the next release. I'm going to produce some hard performance numbers soon as well.
Since HTTP/2 does not have the same performance characteristics/browser limit that HTTP/1.1 does, throttling to 6 requests per server does not make sense. However, disabling throttling completely is also a bad idea because we may end up making too many requests and throwing some away (for example camera flights loading terrain that is no longer in view).
As a compromise, I arrived at 18 being a good request limit. I chose this number because many HTTP/1.1 data providers in Cesium use a/b/c subdomains to work around browser limits, so 3 * 6 = 18.
By default, I've added the Cesium ion servers to this list, but we can and should add other known/popular HTTP/2 servers used by Cesium in the future. We'll eventually make this public whenever the rest of the RequestScheduler goes public.