Skip to content

Commit 6920cbd

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 b264d39 commit 6920cbd

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
@@ -513,12 +513,19 @@ func go_frankenphp_set_vars(threadIndex C.uintptr_t, varsPtr unsafe.Pointer, old
513513
}
514514

515515
// go_frankenphp_get_vars resolves the named worker through the lookup
516-
// (named or catch-all), checks its sk.ready without starting the worker,
517-
// and copies its vars into the return value. If the caller hasn't called
516+
// (named or catch-all), checks sk.ready without starting the worker, and
517+
// copies its vars into the return value. If the caller hasn't called
518518
// ensure() first, this returns a "not ready" error.
519519
//
520+
// callerVersion / outVersion implement a per-request cache:
521+
// - If callerVersion is non-nil and equals the current varsVersion,
522+
// the copy is skipped; outVersion is still set so the C side can
523+
// reuse its cached zval with pointer equality.
524+
// - Otherwise returnValue receives a fresh deep copy and outVersion
525+
// reports the version that copy corresponds to.
526+
//
520527
//export go_frankenphp_get_vars
521-
func go_frankenphp_get_vars(threadIndex C.uintptr_t, name *C.char, nameLen C.size_t, returnValue *C.zval) *C.char {
528+
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 {
522529
thread := phpThreads[threadIndex]
523530
lookup := getLookup(thread)
524531
if lookup == nil {
@@ -544,8 +551,21 @@ func go_frankenphp_get_vars(threadIndex C.uintptr_t, name *C.char, nameLen C.siz
544551
return C.CString("background worker not ready: " + goName + " (no set_vars call yet)")
545552
}
546553

554+
// Fast path: caller's cached version matches current. Skip the copy;
555+
// the caller will reuse its cached zval.
556+
if callerVersion != nil && outVersion != nil {
557+
v := sk.varsVersion.Load()
558+
*outVersion = C.uint64_t(v)
559+
if uint64(*callerVersion) == v {
560+
return nil
561+
}
562+
}
563+
547564
sk.mu.RLock()
548565
C.frankenphp_copy_persistent_vars(returnValue, sk.varsPtr)
566+
if outVersion != nil {
567+
*outVersion = C.uint64_t(sk.varsVersion.Load())
568+
}
549569
sk.mu.RUnlock()
550570

551571
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; }
@@ -993,13 +1018,48 @@ PHP_FUNCTION(frankenphp_get_vars) {
9931018
Z_PARAM_STR(name);
9941019
ZEND_PARSE_PARAMETERS_END();
9951020

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

10051065
PHP_FUNCTION(frankenphp_ensure_background_worker) {
@@ -1652,6 +1712,11 @@ static void *php_thread(void *arg) {
16521712
* so background-worker boot failures can surface the cause. */
16531713
frankenphp_capture_last_php_error();
16541714

1715+
/* Invalidate the per-request get_vars cache before php_request_shutdown
1716+
* tears down request memory: the cached zvals live in request memory
1717+
* and can't be freed after shutdown runs. */
1718+
bg_vars_cache_reset();
1719+
16551720
/* shutdown the request, potential bailout to zend_catch */
16561721
php_request_shutdown((void *)0);
16571722
frankenphp_free_request_context();
@@ -1671,6 +1736,7 @@ static void *php_thread(void *arg) {
16711736
if (!has_attempted_shutdown) {
16721737
/* php_request_shutdown() was not called, force a shutdown now */
16731738
reset_sandboxed_environment();
1739+
bg_vars_cache_reset();
16741740
zend_try { php_request_shutdown((void *)0); }
16751741
zend_catch {}
16761742
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)