Skip to content

CACHE-13371: more cache knobs under request.cf object#6300

Open
yj7o5 wants to merge 1 commit intocloudflare:mainfrom
yj7o5:yawar/CACHE-13371_add-cf-cache-control-field
Open

CACHE-13371: more cache knobs under request.cf object#6300
yj7o5 wants to merge 1 commit intocloudflare:mainfrom
yj7o5:yawar/CACHE-13371_add-cf-cache-control-field

Conversation

@yj7o5
Copy link
Contributor

@yj7o5 yj7o5 commented Mar 11, 2026

This PR adds new cache fields under the request cf object to allow more fine grained control to workers fetch API for caching assets.

@yj7o5 yj7o5 force-pushed the yawar/CACHE-13371_add-cf-cache-control-field branch from cea6afc to 7d0e0f9 Compare March 11, 2026 18:06
@yj7o5 yj7o5 marked this pull request as ready for review March 11, 2026 18:07
@yj7o5 yj7o5 requested review from a team as code owners March 11, 2026 18:07
Copy link
Collaborator

@danlapid danlapid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add new code behind an featureFlags.getWorkerdExperimental() check until we validated that it works as we want in prod, then we can delete the gate.

@yj7o5 yj7o5 force-pushed the yawar/CACHE-13371_add-cf-cache-control-field branch 2 times, most recently from 2bfbff4 to ac4c657 Compare March 11, 2026 20:44
@yj7o5 yj7o5 force-pushed the yawar/CACHE-13371_add-cf-cache-control-field branch 2 times, most recently from ca0cc98 to 56c519b Compare March 12, 2026 22:23
( name = "CACHE_ENABLED", json = "false" ),
],
compatibilityFlags = ["nodejs_compat", "service_binding_extra_handlers", "cache_option_disabled", "url_standard", "workers_api_getters_setters_on_prototype", "fetch_legacy_url", "web_socket_auto_reply_to_close"],
compatibilityFlags = ["nodejs_compat", "service_binding_extra_handlers", "cache_option_disabled", "url_standard", "workers_api_getters_setters_on_prototype", "fetch_legacy_url", "web_socket_auto_reply_to_close", "workerd_experimental"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't work, are you sure the tests are testing what you want?
It should need to be

Suggested change
compatibilityFlags = ["nodejs_compat", "service_binding_extra_handlers", "cache_option_disabled", "url_standard", "workers_api_getters_setters_on_prototype", "fetch_legacy_url", "web_socket_auto_reply_to_close", "workerd_experimental"],
compatibilityFlags = ["nodejs_compat", "service_binding_extra_handlers", "cache_option_disabled", "url_standard", "workers_api_getters_setters_on_prototype", "fetch_legacy_url", "web_socket_auto_reply_to_close", "experimental"],

@danlapid danlapid requested a review from harrishancock March 12, 2026 22:49
@yj7o5 yj7o5 force-pushed the yawar/CACHE-13371_add-cf-cache-control-field branch from 56c519b to 3096570 Compare March 12, 2026 22:58
} else {
obj.set(js, "cacheTtl", js.num(NOCACHE_TTL));
v8::Local<v8::Value> ttlHandle = ttlVal;
auto ttl = jsg::check(ttlHandle->IntegerValue(js.v8Context()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check for negative numbers here? The comment on line 758 suggests a contract.

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 13, 2026

Merging this PR will degrade performance by 11.15%

❌ 1 regressed benchmark
✅ 69 untouched benchmarks
⏩ 129 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
Encode_ASCII_32[TextEncoder][0/0/32] 3.1 ms 3.5 ms -11.15%

Comparing yj7o5:yawar/CACHE-13371_add-cf-cache-control-field (3096570) with main (329f6d4)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants