Skip to content

Commit 2c8dda5

Browse files
feat: per-request cache for frankenphp_get_vars
Ninth step on top of #2287's split. Adds a C-side per-request cache keyed on the background worker's vars version so repeated get_vars reads within one request run at O(1) and return the same HashTable pointer. - __thread HashTable *bg_vars_cache maps worker name -> { version, cached_zval }. Initialized lazily on first get_vars call per request. Destroyed before php_request_shutdown tears down request memory, so the cached zvals are torn down while their backing request-memory structures are still alive. - go_frankenphp_get_vars grew callerVersion / outVersion out-params: - If callerVersion matches the live varsVersion, Go skips the deep copy entirely and only reports outVersion. The C side reuses its cached zval (with ZVAL_COPY for refcount bump). - If versions differ, Go runs the normal copy-under-RLock path and reports the fresh version for the caller to cache. - PHP_FUNCTION(frankenphp_get_vars) consults the cache before calling Go, then either reuses the cached zval (hit) or stores the fresh copy (miss). Identity is preserved: $vars === $prev_vars holds across reads within one request. - TestGetVarsCacheIdentity: two reads in one request return the same zval (=== true). - TestGetVarsCacheManyReads: 500 reads in one script complete without memory corruption, proving the cache tear-down at request end is correct. All 16 existing bg worker tests still pass.
1 parent 59b9278 commit 2c8dda5

5 files changed

Lines changed: 181 additions & 5 deletions

File tree

bgworker.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -462,12 +462,19 @@ func go_frankenphp_set_vars(threadIndex C.uintptr_t, varsPtr unsafe.Pointer, old
462462
}
463463

464464
// go_frankenphp_get_vars resolves the named worker through the lookup
465-
// (named or catch-all), checks its sk.ready without starting the worker,
466-
// and copies its vars into the return value. If the caller hasn't called
465+
// (named or catch-all), checks sk.ready without starting the worker, and
466+
// copies its vars into the return value. If the caller hasn't called
467467
// ensure() first, this returns a "not ready" error.
468468
//
469+
// callerVersion / outVersion implement a per-request cache:
470+
// - If callerVersion is non-nil and equals the current varsVersion,
471+
// the copy is skipped; outVersion is still set so the C side can
472+
// reuse its cached zval with pointer equality.
473+
// - Otherwise returnValue receives a fresh deep copy and outVersion
474+
// reports the version that copy corresponds to.
475+
//
469476
//export go_frankenphp_get_vars
470-
func go_frankenphp_get_vars(threadIndex C.uintptr_t, name *C.char, nameLen C.size_t, returnValue *C.zval) *C.char {
477+
func go_frankenphp_get_vars(threadIndex C.uintptr_t, name *C.char, nameLen C.size_t, returnValue *C.zval, callerVersion *C.uint64_t, outVersion *C.uint64_t) *C.char {
471478
thread := phpThreads[threadIndex]
472479
lookup := getLookup(thread)
473480
if lookup == nil {
@@ -493,8 +500,21 @@ func go_frankenphp_get_vars(threadIndex C.uintptr_t, name *C.char, nameLen C.siz
493500
return C.CString("background worker not ready: " + goName + " (no set_vars call yet)")
494501
}
495502

503+
// Fast path: caller's cached version matches current. Skip the copy;
504+
// the caller will reuse its cached zval.
505+
if callerVersion != nil && outVersion != nil {
506+
v := sk.varsVersion.Load()
507+
*outVersion = C.uint64_t(v)
508+
if uint64(*callerVersion) == v {
509+
return nil
510+
}
511+
}
512+
496513
sk.mu.RLock()
497514
C.frankenphp_copy_persistent_vars(returnValue, sk.varsPtr)
515+
if outVersion != nil {
516+
*outVersion = C.uint64_t(sk.varsVersion.Load())
517+
}
498518
sk.mu.RUnlock()
499519

500520
return nil

bgworkercache_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package frankenphp_test
2+
3+
import (
4+
"os"
5+
"strings"
6+
"testing"
7+
8+
"github.com/dunglas/frankenphp"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
// TestGetVarsCacheIdentity verifies that two get_vars calls within one
14+
// request return the *same* zval (pointer identity via ===) when the
15+
// worker hasn't published a new version in between. This is the user-
16+
// visible guarantee that proves the per-request cache is wired.
17+
func TestGetVarsCacheIdentity(t *testing.T) {
18+
testDataDir := setupFrankenPHP(t,
19+
frankenphp.WithWorkers("cache-worker", "testdata/background-worker-cache-fixture.php", 1,
20+
frankenphp.WithWorkerBackground()),
21+
frankenphp.WithNumThreads(3),
22+
)
23+
24+
body := serveBody(t, testDataDir, "background-worker-cache-identity.php")
25+
assert.Contains(t, body, "first=cached-value")
26+
assert.Contains(t, body, "second=cached-value")
27+
assert.Contains(t, body, "identical=true", "cached zvals must be === across repeated reads:\n"+body)
28+
}
29+
30+
// TestGetVarsCacheManyReads exercises the cache path under load: one
31+
// request calls get_vars 500 times against a nested-array worker. The
32+
// second call onward is a cache hit; the test just asserts the script
33+
// completes without corruption.
34+
func TestGetVarsCacheManyReads(t *testing.T) {
35+
testDataDir := setupFrankenPHP(t,
36+
frankenphp.WithWorkers("cache-worker", "testdata/background-worker-cache-fixture.php", 1,
37+
frankenphp.WithWorkerBackground()),
38+
frankenphp.WithNumThreads(3),
39+
)
40+
41+
// ensure() first so the eager-start race doesn't surface before the
42+
// 500-read loop even begins.
43+
php := `<?php
44+
frankenphp_ensure_background_worker('cache-worker');
45+
for ($i = 0; $i < 500; $i++) {
46+
$vars = frankenphp_get_vars('cache-worker');
47+
}
48+
echo 'ok=', $vars['marker'] ?? 'MISSING', "\n";`
49+
tmp := testDataDir + "bg-cache-many.php"
50+
require.NoError(t, os.WriteFile(tmp, []byte(php), 0644))
51+
t.Cleanup(func() { _ = os.Remove(tmp) })
52+
53+
body := serveBody(t, testDataDir, "bg-cache-many.php")
54+
assert.Contains(t, body, "ok=cached-value", "500 cached reads should all succeed:\n"+body)
55+
assert.False(t, strings.Contains(body, "Fatal error") || strings.Contains(body, "corrupted"), "no corruption expected:\n"+body)
56+
}

frankenphp.c

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,31 @@ __thread php_stream *worker_signaling_stream = NULL;
9393
__thread char *captured_last_php_error = NULL;
9494
__thread HashTable *sandboxed_env = NULL;
9595

96+
/* Per-thread cache for frankenphp_get_vars results. Maps worker name to
97+
* { version, cached_zval }. When the Go side reports the version hasn't
98+
* changed, the cached zval is returned with a refcount bump, giving the
99+
* PHP caller the same HashTable pointer so repeated reads within a
100+
* request run at O(1) without walking persistent memory every time. */
101+
typedef struct {
102+
uint64_t version;
103+
zval value;
104+
} bg_vars_cache_entry;
105+
__thread HashTable *bg_vars_cache = NULL;
106+
107+
static void bg_vars_cache_dtor(zval *zv) {
108+
bg_vars_cache_entry *entry = Z_PTR_P(zv);
109+
zval_ptr_dtor(&entry->value);
110+
free(entry);
111+
}
112+
113+
static void bg_vars_cache_reset(void) {
114+
if (bg_vars_cache) {
115+
zend_hash_destroy(bg_vars_cache);
116+
free(bg_vars_cache);
117+
bg_vars_cache = NULL;
118+
}
119+
}
120+
96121
#ifndef PHP_WIN32
97122
static bool is_forked_child = false;
98123
static void frankenphp_fork_child(void) { is_forked_child = true; }
@@ -996,13 +1021,48 @@ PHP_FUNCTION(frankenphp_get_vars) {
9961021
Z_PARAM_STR(name);
9971022
ZEND_PARSE_PARAMETERS_END();
9981023

999-
char *error = go_frankenphp_get_vars(thread_index, (char *)ZSTR_VAL(name),
1000-
ZSTR_LEN(name), return_value);
1024+
/* Look up a cached entry first so Go can short-circuit the copy when
1025+
* the background worker has not changed its vars since we last read
1026+
* them. On a cache hit we reuse the same zval, so $vars === $prev_vars
1027+
* holds across repeated reads within one request. */
1028+
uint64_t caller_version = 0;
1029+
uint64_t out_version = 0;
1030+
bg_vars_cache_entry *cached = NULL;
1031+
if (bg_vars_cache) {
1032+
zval *entry_zv = zend_hash_find(bg_vars_cache, name);
1033+
if (entry_zv) {
1034+
cached = Z_PTR_P(entry_zv);
1035+
caller_version = cached->version;
1036+
}
1037+
}
1038+
1039+
char *error = go_frankenphp_get_vars(
1040+
thread_index, (char *)ZSTR_VAL(name), ZSTR_LEN(name), return_value,
1041+
cached ? &caller_version : NULL, &out_version);
10011042
if (error) {
10021043
zend_throw_exception(spl_ce_RuntimeException, error, 0);
10031044
free(error);
10041045
RETURN_THROWS();
10051046
}
1047+
1048+
if (cached && out_version == caller_version) {
1049+
/* Cache hit: Go skipped the copy. Return the cached zval. */
1050+
ZVAL_COPY(return_value, &cached->value);
1051+
return;
1052+
}
1053+
1054+
/* Cache miss: store the fresh copy so subsequent reads within this
1055+
* request can be served without walking persistent memory. */
1056+
if (!bg_vars_cache) {
1057+
bg_vars_cache = malloc(sizeof(HashTable));
1058+
zend_hash_init(bg_vars_cache, 4, NULL, bg_vars_cache_dtor, 1);
1059+
}
1060+
bg_vars_cache_entry *entry = malloc(sizeof(*entry));
1061+
entry->version = out_version;
1062+
ZVAL_COPY(&entry->value, return_value);
1063+
zval entry_zv;
1064+
ZVAL_PTR(&entry_zv, entry);
1065+
zend_hash_update(bg_vars_cache, name, &entry_zv);
10061066
}
10071067

10081068
PHP_FUNCTION(frankenphp_ensure_background_worker) {
@@ -1655,6 +1715,11 @@ static void *php_thread(void *arg) {
16551715
* so background-worker boot failures can surface the cause. */
16561716
frankenphp_capture_last_php_error();
16571717

1718+
/* Invalidate the per-request get_vars cache before php_request_shutdown
1719+
* tears down request memory: the cached zvals live in request memory
1720+
* and can't be freed after shutdown runs. */
1721+
bg_vars_cache_reset();
1722+
16581723
/* shutdown the request, potential bailout to zend_catch */
16591724
php_request_shutdown((void *)0);
16601725
frankenphp_free_request_context();
@@ -1674,6 +1739,7 @@ static void *php_thread(void *arg) {
16741739
if (!has_attempted_shutdown) {
16751740
/* php_request_shutdown() was not called, force a shutdown now */
16761741
reset_sandboxed_environment();
1742+
bg_vars_cache_reset();
16771743
zend_try { php_request_shutdown((void *)0); }
16781744
zend_catch {}
16791745
zend_end_try();
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
// Long-lived bg worker: disable PHP max_execution_time so the 30s default
4+
// cannot interrupt the stream_select park. The C side calls
5+
// zend_unset_timeout() too, but the belt-and-suspenders here covers PHP
6+
// builds where that path does not fully disarm the timer.
7+
set_time_limit(0);
8+
9+
// Stable bg worker: publishes one snapshot and never updates.
10+
frankenphp_set_vars([
11+
'marker' => 'cached-value',
12+
]);
13+
14+
$stream = frankenphp_get_worker_handle();
15+
if ($stream !== null) {
16+
$read = [$stream];
17+
$write = null;
18+
$except = null;
19+
stream_select($read, $write, $except, null);
20+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
// Calls get_vars twice within one request. The cache guarantees pointer
4+
// identity: the two zvals should be === (same HashTable pointer) because
5+
// the underlying worker hasn't published a new version in between.
6+
// ensure() waits for the bg worker's first set_vars so eager-start races
7+
// don't surface before the cache path is exercised.
8+
frankenphp_ensure_background_worker('cache-worker');
9+
$first = frankenphp_get_vars('cache-worker');
10+
$second = frankenphp_get_vars('cache-worker');
11+
12+
echo 'first=', isset($first['marker']) ? $first['marker'] : 'MISSING', "\n";
13+
echo 'second=', isset($second['marker']) ? $second['marker'] : 'MISSING', "\n";
14+
echo 'identical=', ($first === $second) ? 'true' : 'false', "\n";

0 commit comments

Comments
 (0)