Skip to content

Commit 1c6cb73

Browse files
TartanLlamaTartanLlama
authored
Make simple cache more async (#1246)
1 parent 89bb9bf commit 1c6cb73

File tree

1 file changed

+92
-43
lines changed

1 file changed

+92
-43
lines changed

‎runtime/fastly/builtins/cache-simple.cpp‎

Lines changed: 92 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ using fastly::FastlyGetErrorMessage;
1717
using fastly::fastly::convertBodyInit;
1818
using fastly::fetch::RequestOrResponse;
1919

20+
namespace {
21+
api::Engine *GLOBAL_ENGINE;
22+
}
23+
2024
namespace fastly::cache_simple {
2125

2226
template <RequestOrResponse::BodyReadResult result_type>
@@ -394,62 +398,39 @@ bool get_or_set_catch_handler(JSContext *cx, JS::HandleObject lookup_state,
394398
return true;
395399
}
396400

397-
} // namespace
398-
399-
// static getOrSet(key: string, set: () => Promise<{value: BodyInit, ttl: number}>):
400-
// SimpleCacheEntry | null; static getOrSet(key: string, set: () => Promise<{value: ReadableStream,
401-
// ttl: number, length: number}>): SimpleCacheEntry | null;
402-
bool SimpleCache::getOrSet(JSContext *cx, unsigned argc, JS::Value *vp) {
403-
REQUEST_HANDLER_ONLY("The SimpleCache builtin");
404-
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
405-
if (!args.requireAtLeast(cx, "SimpleCache.getOrSet", 2)) {
406-
return false;
407-
}
408-
409-
// Convert key parameter into a string and check the value adheres to our validation rules.
410-
auto key_chars = core::encode(cx, args.get(0));
411-
if (!key_chars) {
401+
bool process_pending_cache_lookup(JSContext *cx, host_api::CacheHandle::Handle handle,
402+
JS::HandleObject context_obj, JS::HandleValue) {
403+
host_api::CacheHandle pending_lookup(handle);
404+
JS::RootedValue key_val(cx);
405+
if (!JS_GetProperty(cx, context_obj, "key", &key_val)) {
412406
return false;
413407
}
414-
415-
if (key_chars.len == 0) {
416-
JS_ReportErrorASCII(cx, "SimpleCache.getOrSet: key can not be an empty string");
408+
auto key_chars = core::encode(cx, key_val);
409+
JS::RootedValue set_function_val(cx);
410+
if (!JS_GetProperty(cx, context_obj, "set_function", &set_function_val)) {
417411
return false;
418412
}
419-
if (key_chars.len > 8135) {
420-
JS_ReportErrorASCII(
421-
cx, "SimpleCache.getOrSet: key is too long, the maximum allowed length is 8135.");
413+
JS::RootedValue promise_val(cx);
414+
if (!JS_GetProperty(cx, context_obj, "promise", &promise_val)) {
422415
return false;
423416
}
417+
JS::RootedObject promise_obj(cx, &promise_val.toObject());
424418

425-
JS::RootedObject promise(cx, JS::NewPromiseObject(cx, nullptr));
426-
if (!promise) {
427-
return ReturnPromiseRejectedWithPendingError(cx, args);
428-
}
429-
430-
auto res = host_api::CacheHandle::transaction_lookup(key_chars, host_api::CacheLookupOptions{});
431-
if (auto *err = res.to_err()) {
432-
HANDLE_ERROR(cx, *err);
433-
return false;
434-
}
435-
436-
auto handle = res.unwrap();
437-
BEGIN_TRANSACTION(transaction, cx, promise, handle);
419+
BEGIN_TRANSACTION(transaction, cx, promise_obj, pending_lookup);
438420

439421
// Check if a fresh cache item was found, if that's the case, then we will resolve
440422
// with a SimpleCacheEntry containing the value. Else, call the content-provided
441423
// function in the `set` parameter and insert it's returned value property into the
442424
// cache under the provided `key`, and then we will resolve with a SimpleCacheEntry
443425
// containing the value.
444-
auto state_res = handle.get_state();
426+
auto state_res = pending_lookup.get_state();
445427
if (auto *err = state_res.to_err()) {
446428
return false;
447429
}
448430

449431
auto state = state_res.unwrap();
450-
args.rval().setObject(*promise);
451432
if (state.is_usable()) {
452-
auto body_res = handle.get_body(host_api::CacheGetBodyOptions{});
433+
auto body_res = pending_lookup.get_body(host_api::CacheGetBodyOptions{});
453434
if (auto *err = body_res.to_err()) {
454435
return false;
455436
}
@@ -461,16 +442,15 @@ bool SimpleCache::getOrSet(JSContext *cx, unsigned argc, JS::Value *vp) {
461442

462443
JS::RootedValue result(cx);
463444
result.setObject(*entry);
464-
JS::ResolvePromise(cx, promise, result);
445+
JS::ResolvePromise(cx, promise_obj, result);
465446
return true;
466447
} else {
467-
auto arg1 = args.get(1);
468-
if (!arg1.isObject() || !JS::IsCallable(&arg1.toObject())) {
448+
if (!set_function_val.isObject() || !JS::IsCallable(&set_function_val.toObject())) {
469449
JS_ReportErrorLatin1(cx, "SimpleCache.getOrSet: set argument is not a function");
470450
return false;
471451
}
472452
JS::RootedValueArray<0> fnargs(cx);
473-
JS::RootedObject fn(cx, &arg1.toObject());
453+
JS::RootedObject fn(cx, &set_function_val.toObject());
474454
JS::RootedValue result(cx);
475455
if (!JS::Call(cx, JS::NullHandleValue, fn, fnargs, &result)) {
476456
return false;
@@ -483,7 +463,7 @@ bool SimpleCache::getOrSet(JSContext *cx, unsigned argc, JS::Value *vp) {
483463

484464
// JS::RootedObject owner(cx, JS_NewPlainObject(cx));
485465
JS::RootedObject lookup_state(cx, JS_NewPlainObject(cx));
486-
JS::RootedValue handle_val(cx, JS::NumberValue(handle.handle));
466+
JS::RootedValue handle_val(cx, JS::NumberValue(handle));
487467
if (!JS_SetProperty(cx, lookup_state, "handle", handle_val)) {
488468
return false;
489469
}
@@ -492,7 +472,7 @@ bool SimpleCache::getOrSet(JSContext *cx, unsigned argc, JS::Value *vp) {
492472
if (!JS_SetProperty(cx, lookup_state, "key", keyVal)) {
493473
return false;
494474
}
495-
JS::RootedValue promise_val(cx, JS::ObjectValue(*promise));
475+
JS::RootedValue promise_val(cx, JS::ObjectValue(*promise_obj));
496476
if (!JS_SetProperty(cx, lookup_state, "promise", promise_val)) {
497477
return false;
498478
}
@@ -517,6 +497,73 @@ bool SimpleCache::getOrSet(JSContext *cx, unsigned argc, JS::Value *vp) {
517497
}
518498
}
519499

500+
} // namespace
501+
502+
// static getOrSet(key: string, set: () => Promise<{value: BodyInit, ttl: number}>):
503+
// SimpleCacheEntry | null; static getOrSet(key: string, set: () => Promise<{value: ReadableStream,
504+
// ttl: number, length: number}>): SimpleCacheEntry | null;
505+
bool SimpleCache::getOrSet(JSContext *cx, unsigned argc, JS::Value *vp) {
506+
REQUEST_HANDLER_ONLY("The SimpleCache builtin");
507+
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
508+
if (!args.requireAtLeast(cx, "SimpleCache.getOrSet", 2)) {
509+
return false;
510+
}
511+
512+
// Convert key parameter into a string and check the value adheres to our validation rules.
513+
auto key_chars = core::encode(cx, args.get(0));
514+
if (!key_chars) {
515+
return false;
516+
}
517+
518+
if (key_chars.len == 0) {
519+
JS_ReportErrorASCII(cx, "SimpleCache.getOrSet: key can not be an empty string");
520+
return false;
521+
}
522+
if (key_chars.len > 8135) {
523+
JS_ReportErrorASCII(
524+
cx, "SimpleCache.getOrSet: key is too long, the maximum allowed length is 8135.");
525+
return false;
526+
}
527+
528+
JS::RootedObject promise(cx, JS::NewPromiseObject(cx, nullptr));
529+
if (!promise) {
530+
return ReturnPromiseRejectedWithPendingError(cx, args);
531+
}
532+
533+
auto res = host_api::CacheHandle::transaction_lookup(key_chars, host_api::CacheLookupOptions{});
534+
if (auto *err = res.to_err()) {
535+
HANDLE_ERROR(cx, *err);
536+
return false;
537+
}
538+
539+
// The async task requires some extra state: the key, the `set` function, and the promise.
540+
// We wrap this all up into one object, so `process_pending_cache_lookup` can retrieve everything.
541+
// This could be avoided with some changes to `FastlyAsyncTask`, see
542+
// (https://github.com/fastly/js-compute-runtime/issues/1245)
543+
JS::RootedObject context_obj(cx, JS_NewPlainObject(cx));
544+
if (!context_obj) {
545+
return false;
546+
}
547+
JS::RootedValue key_val(cx, args.get(0));
548+
if (!JS_SetProperty(cx, context_obj, "key", key_val)) {
549+
return false;
550+
}
551+
JS::RootedValue set_func_val(cx, args.get(1));
552+
if (!JS_SetProperty(cx, context_obj, "set_function", set_func_val)) {
553+
return false;
554+
}
555+
JS::RootedValue promise_val(cx, JS::ObjectValue(*promise));
556+
if (!JS_SetProperty(cx, context_obj, "promise", promise_val)) {
557+
return false;
558+
}
559+
auto handle = res.unwrap();
560+
GLOBAL_ENGINE->queue_async_task(new FastlyAsyncTask(
561+
handle.handle, context_obj, JS::UndefinedHandleValue, process_pending_cache_lookup));
562+
563+
args.rval().setObject(*promise);
564+
return true;
565+
}
566+
520567
// static set(key: string, value: BodyInit, ttl: number): undefined;
521568
// static set(key: string, value: ReadableStream, ttl: number, length: number): undefined;
522569
bool SimpleCache::set(JSContext *cx, unsigned argc, JS::Value *vp) {
@@ -791,6 +838,8 @@ const JSPropertySpec SimpleCache::properties[] = {
791838
JS_STRING_SYM_PS(toStringTag, "SimpleCache", JSPROP_READONLY), JS_PS_END};
792839

793840
bool install(api::Engine *engine) {
841+
GLOBAL_ENGINE = engine;
842+
794843
if (!SimpleCacheEntry::init_class_impl(engine->cx(), engine->global())) {
795844
return false;
796845
}

0 commit comments

Comments
 (0)