feat: basic etag handling for tile serving#1834
feat: basic etag handling for tile serving#1834CommanderStorm merged 12 commits intomaplibre:mainfrom
Conversation
martin/src/srv/tiles.rs
Outdated
| zoom: Option<u8>, | ||
| query: &'a str, | ||
| accept_enc: Option<AcceptEncoding>, | ||
| (accept_enc, if_none_match): (Option<AcceptEncoding>, Option<IfNoneMatch>), |
There was a problem hiding this comment.
this is done like this to not make clippy angry that these are defintively too many arguments.
There was a problem hiding this comment.
let's just add the #[expect(...)] here - much better than to be confused about the meaning of the code
martin/src/srv/tiles.rs
Outdated
|
|
||
| pub async fn get_http_response(&self, xyz: TileCoord) -> ActixResult<HttpResponse> { | ||
| let tile = self.get_tile_content(xyz).await?; | ||
| let hash = xxhash_rust::xxh3::xxh3_128(&tile.data); |
There was a problem hiding this comment.
In a future PR: plumb the hash from mbtiles and pg through the caching and merging in get_tile_content and below.
|
So basically no performance impact. Brings up the point: Do we really want to use the hashes from the data? |
|
i would prefer to use the hash when its there. I was looking at the perf reports above, and it seems they are not aligned in the 2nd and 3rd row? Not sure which was which. The number of requests/sec is probably the most telling due to each request being served too fast. |
|
Fixed the benchmarks. The essence is that current version performs 5% worse in terms of Requests per second. Regarding the using the hash when its there, that is something that I plan to do, just not in this PR. The reason is that that change snakes itsself through a lot of the codebase somehow. |
Co-authored-by: Lucas <zhangyijunmetro@hotmail.com>
for more information, see https://pre-commit.ci
|
thx! |
This PR adds etag handling via an actix wrap instruction to all "non-tile" endpoints. I ommitted some endpoints which I suspect are not called more than once such as `/` if the webui is not compiled in. The reason for not being smarter for some is that honestly, the impact of this is less than I expected based on benchmarks in #1834 and that `actix-middleware-etag` uses the same hash algorithm. <del> I have not addded tests for this as frankly, this does not seem like a feature that needs deep tests. </del> By request, this PR now includes all stemi-static headers. --------- Co-authored-by: Yuri Astrakhan <yuriastrakhan@gmail.com> Co-authored-by: sharkAndshark <zhangyijunmetro@hotmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
3 participants
This PR adds basic etag handling to the tile serving.
I am currently using an okay hash function for this.
There are better ones like
gxhash, but for compatability with thembtiles-crate, usingxxhashis likely also fine. Also,xxhashdoes not require SIMD.benchmarks (click to expand)
This is the next PR in the chain started at #1787