Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase Request Scheduler limits to better support HTTP/2 and above by default #11627

Closed
ggetz opened this issue Nov 13, 2023 · 5 comments · Fixed by #11673
Closed

Increase Request Scheduler limits to better support HTTP/2 and above by default #11627

ggetz opened this issue Nov 13, 2023 · 5 comments · Fixed by #11673
Assignees

Comments

@ggetz
Copy link
Contributor

ggetz commented Nov 13, 2023

Right now RequestScheduler limits simultaneous requests from unknown servers to 6. This was originally put in place because of limitations of HTTP/1.

For HTTP/2 connections and above, this restriction is no longer needed, and now becomes a bottleneck. This can hinder performance for 3D Tiles, Imagery, Terrain and more.

RequestScheduler maintains a list of servers we know are HTTP/2 or greater, such as Cesium ion, to workaround this restriction.

However, with most web traffic now being HTTP/2 and above, it may be time to reconsider the default entirely.

@mramato
Copy link
Contributor

mramato commented Nov 21, 2023

I think we're at the point where the current limitation does more harm than good. HTTP/2 and HTTP/3 are now both widely used. HTTP/1.1 is now the exception.

Unfortunately, there's still no way (that I know of) to detect what protocol a server is using from the client; so my suggestion is we raise the default limit to something reasonable (I have no idea if the 18 we have in place makes sense in modern Cesium, we should do some performance testing and evaluate, I suspect it hurts performance) and then allow users to lower it for a server if they want (though I would argue we don't need this at all).

We want CesiumJS to be fast by default and I suspect that's not the case for most users outside of those specifically listed in RequestSchedule. Also, for products like Cesium ion Self-Hosted, which have no predefined URL, it's not reasonable to expect users to have to update their apps to add a list of known urls.

@csciguy8
Copy link

csciguy8 commented Nov 21, 2023

For reference, cesium-unreal is using libcurl, and to the best of our knowledge, is stuck at HTTP 1.1 right now.

The number of simultaneous requests for tilesets defaults to 20. In my location, increasing this value doesn't yield much more performance. Reducing it can drastically slow down load times.

There's even an automated test that runs a series of passes to determine what the fastest value is.

@ggetz
Copy link
Contributor Author

ggetz commented Nov 21, 2023

I have no idea if the 18 we have in place makes sense in modern Cesium, we should do some performance testing and evaluate, I suspect it hurts performance

@jjhembd's recent performance testing yielded similar results. Raising this value did not result in performance gains due to bottlenecks in other places in the pipeline.

@mramato
Copy link
Contributor

mramato commented Nov 21, 2023

@ggetz That's good to hear, are there any plans to address the performance issues in those areas? It seems crazy to me that bandwidth is not the limiting factor.

@ggetz
Copy link
Contributor Author

ggetz commented Nov 21, 2023

No immediate plans to execute, but top of mind for 3D Tile performance is:

  • Profiling glTF loading to ID bottlenecks
  • Re-evaluate tileset traversal
  • Horizon culling
  • Occlusion culling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants