Prevent ETag generation when Cache-Control: no-store is set#6994
Prevent ETag generation when Cache-Control: no-store is set#6994GroophyLifefor wants to merge 1 commit intoexpressjs:masterfrom
Conversation
jonchurch
left a comment
There was a problem hiding this comment.
This ensures compliance with HTTP caching specifications where no-store prohibits storing the response.
This is not accurate as written. This PR is a perf optimization not a spec alignment. ETAGs can always be sent, Cache-Control: no-store just makes them useless to the client.
Do we have benchmark data showing if this creates a meaningful impact? We pay for a (fast) header check on every response to see if we can save the (also fast) hashing of the body.
|
I don't think it makes meaningful impact but I'll look asap. |
|
We should also review what Doug had mentioned, i haven’t done my investigation on this yet. |
Based on mine measurements, the optimization reduces res.send from ~0.20 ms to ~0.16 ms per request. So it does improve performance, mainly by removing ETag computation, but the overall impact is very small that around 0.04 ms per response. |
Doug's comment is probably based on the
I have looked at Firefox source code and it looks like Based on this, I'd say that ETag is not necessary, but RFC 9111 introduced one more change. At the end of
The Section 5.2.2.3. must-understand says:
If I understand this correctly, |
Respect the Cache-Control: no-store directive by skipping ETag generation when this header is present.
Changes:
This ensures compliance with HTTP caching specifications where no-store prohibits storing the response.
Issue: #2472
Review wanted to @bjohansebas