Skip to content

Commit 98354c1

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 a5abd16 commit 98354c1

5 files changed

Lines changed: 208 additions & 5 deletions

File tree

background_worker_cache_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package frankenphp_test
2+
3+
import (
4+
"errors"
5+
"io"
6+
"net/http/httptest"
7+
"os"
8+
"strings"
9+
"testing"
10+
11+
"github.com/dunglas/frankenphp"
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
// TestGetVarsCacheIdentity verifies that two get_vars calls within one
17+
// request return the *same* zval (pointer identity via ===) when the
18+
// worker hasn't published a new version in between. This is the user-
19+
// visible guarantee that proves the per-request cache is wired.
20+
func TestGetVarsCacheIdentity(t *testing.T) {
21+
cwd, _ := os.Getwd()
22+
testDataDir := cwd + "/testdata/"
23+
24+
require.NoError(t, frankenphp.Init(
25+
frankenphp.WithWorkers("cache-worker", testDataDir+"background-worker-cache-fixture.php", 1,
26+
frankenphp.WithWorkerBackground()),
27+
frankenphp.WithNumThreads(3),
28+
))
29+
t.Cleanup(frankenphp.Shutdown)
30+
31+
req := httptest.NewRequest("GET", "http://example.com/background-worker-cache-identity.php", nil)
32+
fr, err := frankenphp.NewRequestWithContext(req, frankenphp.WithRequestDocumentRoot(testDataDir, false))
33+
require.NoError(t, err)
34+
35+
w := httptest.NewRecorder()
36+
if err := frankenphp.ServeHTTP(w, fr); err != nil && !errors.As(err, &frankenphp.ErrRejected{}) {
37+
t.Fatalf("serve: %v", err)
38+
}
39+
body, _ := io.ReadAll(w.Result().Body)
40+
out := string(body)
41+
42+
assert.Contains(t, out, "first=cached-value")
43+
assert.Contains(t, out, "second=cached-value")
44+
assert.Contains(t, out, "identical=true", "cached zvals must be === across repeated reads:\n"+out)
45+
}
46+
47+
// TestGetVarsCacheManyReads exercises the cache path under load: one
48+
// request calls get_vars 500 times against a nested-array worker. The
49+
// second call onward is a cache hit; the test just asserts the script
50+
// completes without corruption.
51+
func TestGetVarsCacheManyReads(t *testing.T) {
52+
cwd, _ := os.Getwd()
53+
testDataDir := cwd + "/testdata/"
54+
55+
require.NoError(t, frankenphp.Init(
56+
frankenphp.WithWorkers("cache-worker", testDataDir+"background-worker-cache-fixture.php", 1,
57+
frankenphp.WithWorkerBackground()),
58+
frankenphp.WithNumThreads(3),
59+
))
60+
t.Cleanup(frankenphp.Shutdown)
61+
62+
// ensure() first so the eager-start race doesn't surface before the
63+
// 500-read loop even begins.
64+
php := `<?php
65+
frankenphp_ensure_background_worker('cache-worker');
66+
for ($i = 0; $i < 500; $i++) {
67+
$vars = frankenphp_get_vars('cache-worker');
68+
}
69+
echo 'ok=', $vars['marker'] ?? 'MISSING', "\n";`
70+
tmp := testDataDir + "bg-cache-many.php"
71+
require.NoError(t, os.WriteFile(tmp, []byte(php), 0644))
72+
t.Cleanup(func() { _ = os.Remove(tmp) })
73+
74+
req := httptest.NewRequest("GET", "http://example.com/bg-cache-many.php", nil)
75+
fr, _ := frankenphp.NewRequestWithContext(req, frankenphp.WithRequestDocumentRoot(testDataDir, false))
76+
w := httptest.NewRecorder()
77+
_ = frankenphp.ServeHTTP(w, fr)
78+
body, _ := io.ReadAll(w.Result().Body)
79+
out := string(body)
80+
81+
assert.Contains(t, out, "ok=cached-value", "500 cached reads should all succeed:\n"+out)
82+
assert.False(t, strings.Contains(out, "Fatal error") || strings.Contains(out, "corrupted"), "no corruption expected:\n"+out)
83+
}

bgworker.go

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

390390
// go_frankenphp_get_vars resolves the named worker through the lookup
391-
// (named or catch-all), checks its sk.ready without starting the worker,
392-
// and copies its vars into the return value. If the caller hasn't called
391+
// (named or catch-all), checks sk.ready without starting the worker, and
392+
// copies its vars into the return value. If the caller hasn't called
393393
// ensure() first, this returns a "not ready" error.
394394
//
395+
// callerVersion / outVersion implement a per-request cache:
396+
// - If callerVersion is non-nil and equals the current varsVersion,
397+
// the copy is skipped; outVersion is still set so the C side can
398+
// reuse its cached zval with pointer equality.
399+
// - Otherwise returnValue receives a fresh deep copy and outVersion
400+
// reports the version that copy corresponds to.
401+
//
395402
//export go_frankenphp_get_vars
396-
func go_frankenphp_get_vars(threadIndex C.uintptr_t, name *C.char, nameLen C.size_t, returnValue *C.zval) *C.char {
403+
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 {
397404
thread := phpThreads[threadIndex]
398405
lookup := getLookup(thread)
399406
if lookup == nil {
@@ -419,8 +426,21 @@ func go_frankenphp_get_vars(threadIndex C.uintptr_t, name *C.char, nameLen C.siz
419426
return C.CString("background worker not ready: " + goName + " (no set_vars call yet)")
420427
}
421428

429+
// Fast path: caller's cached version matches current. Skip the copy;
430+
// the caller will reuse its cached zval.
431+
if callerVersion != nil && outVersion != nil {
432+
v := sk.varsVersion.Load()
433+
*outVersion = C.uint64_t(v)
434+
if uint64(*callerVersion) == v {
435+
return nil
436+
}
437+
}
438+
422439
sk.mu.RLock()
423440
C.frankenphp_copy_persistent_vars(returnValue, sk.varsPtr)
441+
if outVersion != nil {
442+
*outVersion = C.uint64_t(sk.varsVersion.Load())
443+
}
424444
sk.mu.RUnlock()
425445

426446
return nil

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; }
@@ -994,13 +1019,48 @@ PHP_FUNCTION(frankenphp_get_vars) {
9941019
Z_PARAM_STR(name);
9951020
ZEND_PARSE_PARAMETERS_END();
9961021

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

10061066
PHP_FUNCTION(frankenphp_ensure_background_worker) {
@@ -1653,6 +1713,11 @@ static void *php_thread(void *arg) {
16531713
* so background-worker boot failures can surface the cause. */
16541714
frankenphp_capture_last_php_error();
16551715

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