Skip to content

Commit 83ef590

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. ## What - __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. ## Tests - 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 f832257 commit 83ef590

5 files changed

Lines changed: 207 additions & 5 deletions

background_worker.go

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -527,11 +527,17 @@ func go_frankenphp_set_vars(threadIndex C.uintptr_t, varsPtr unsafe.Pointer, old
527527

528528
// go_frankenphp_get_vars resolves the named worker through the lookup
529529
// (named or catch-all), waits on sk.ready without starting the worker,
530-
// and copies its vars into the return value. If the caller hasn't called
531-
// ensure() first, this returns a "not ready" error.
530+
// and copies its vars into the return value.
531+
//
532+
// callerVersion / outVersion implement a per-request cache:
533+
// - If callerVersion is non-nil and equals the current varsVersion,
534+
// the copy is skipped; outVersion is still set so the C side can
535+
// reuse its cached zval with pointer equality.
536+
// - Otherwise returnValue receives a fresh deep copy and outVersion
537+
// reports the version that copy corresponds to.
532538
//
533539
//export go_frankenphp_get_vars
534-
func go_frankenphp_get_vars(threadIndex C.uintptr_t, name *C.char, nameLen C.size_t, returnValue *C.zval) *C.char {
540+
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 {
535541
thread := phpThreads[threadIndex]
536542
lookup := getLookup(thread)
537543
if lookup == nil {
@@ -556,8 +562,21 @@ func go_frankenphp_get_vars(threadIndex C.uintptr_t, name *C.char, nameLen C.siz
556562
return C.CString("background worker not ready: " + goName + " (no set_vars call yet)")
557563
}
558564

565+
// Fast path: caller's cached version matches current. Skip the copy;
566+
// the caller will reuse its cached zval.
567+
if callerVersion != nil && outVersion != nil {
568+
v := sk.varsVersion.Load()
569+
*outVersion = C.uint64_t(v)
570+
if uint64(*callerVersion) == v {
571+
return nil
572+
}
573+
}
574+
559575
sk.mu.RLock()
560576
C.frankenphp_copy_persistent_vars(returnValue, sk.varsPtr)
577+
if outVersion != nil {
578+
*outVersion = C.uint64_t(sk.varsVersion.Load())
579+
}
561580
sk.mu.RUnlock()
562581

563582
return nil

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+
}

frankenphp.c

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

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

1035-
char *error = go_frankenphp_get_vars(thread_index, (char *)ZSTR_VAL(name),
1036-
ZSTR_LEN(name), return_value);
1060+
/* Look up a cached entry first so Go can short-circuit the copy when
1061+
* the background worker has not changed its vars since we last read
1062+
* them. On a cache hit we reuse the same zval, so $vars === $prev_vars
1063+
* holds across repeated reads within one request. */
1064+
uint64_t caller_version = 0;
1065+
uint64_t out_version = 0;
1066+
bg_vars_cache_entry *cached = NULL;
1067+
if (bg_vars_cache) {
1068+
zval *entry_zv = zend_hash_find(bg_vars_cache, name);
1069+
if (entry_zv) {
1070+
cached = Z_PTR_P(entry_zv);
1071+
caller_version = cached->version;
1072+
}
1073+
}
1074+
1075+
char *error = go_frankenphp_get_vars(
1076+
thread_index, (char *)ZSTR_VAL(name), ZSTR_LEN(name), return_value,
1077+
cached ? &caller_version : NULL, &out_version);
10371078
if (error) {
10381079
zend_throw_exception(spl_ce_RuntimeException, error, 0);
10391080
free(error);
10401081
RETURN_THROWS();
10411082
}
1083+
1084+
if (cached && out_version == caller_version) {
1085+
/* Cache hit: Go skipped the copy. Return the cached zval. */
1086+
ZVAL_COPY(return_value, &cached->value);
1087+
return;
1088+
}
1089+
1090+
/* Cache miss: store the fresh copy so subsequent reads within this
1091+
* request can be served without walking persistent memory. */
1092+
if (!bg_vars_cache) {
1093+
bg_vars_cache = malloc(sizeof(HashTable));
1094+
zend_hash_init(bg_vars_cache, 4, NULL, bg_vars_cache_dtor, 1);
1095+
}
1096+
bg_vars_cache_entry *entry = malloc(sizeof(*entry));
1097+
entry->version = out_version;
1098+
ZVAL_COPY(&entry->value, return_value);
1099+
zval entry_zv;
1100+
ZVAL_PTR(&entry_zv, entry);
1101+
zend_hash_update(bg_vars_cache, name, &entry_zv);
10421102
}
10431103

10441104
PHP_FUNCTION(frankenphp_ensure_background_worker) {
@@ -1673,6 +1733,11 @@ static void *php_thread(void *arg) {
16731733
* so background-worker boot failures can surface the cause. */
16741734
frankenphp_capture_last_php_error();
16751735

1736+
/* Invalidate the per-request get_vars cache before php_request_shutdown
1737+
* tears down request memory: the cached zvals live in request memory
1738+
* and can't be freed after shutdown runs. */
1739+
bg_vars_cache_reset();
1740+
16761741
/* shutdown the request, potential bailout to zend_catch */
16771742
php_request_shutdown((void *)0);
16781743
frankenphp_free_request_context();
@@ -1692,6 +1757,7 @@ static void *php_thread(void *arg) {
16921757
if (!has_attempted_shutdown) {
16931758
/* php_request_shutdown() was not called, force a shutdown now */
16941759
reset_sandboxed_environment();
1760+
bg_vars_cache_reset();
16951761
zend_try { php_request_shutdown((void *)0); }
16961762
zend_catch {}
16971763
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)