Skip to content

Commit ac54f95

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 21f7fa7 commit ac54f95

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

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

551+
// Fast path: caller's cached version matches current. Skip the copy;
552+
// the caller will reuse its cached zval.
553+
if callerVersion != nil && outVersion != nil {
554+
v := sk.varsVersion.Load()
555+
*outVersion = C.uint64_t(v)
556+
if uint64(*callerVersion) == v {
557+
return nil
558+
}
559+
}
560+
545561
sk.mu.RLock()
546562
C.frankenphp_copy_persistent_vars(returnValue, sk.varsPtr)
563+
if outVersion != nil {
564+
*outVersion = C.uint64_t(sk.varsVersion.Load())
565+
}
547566
sk.mu.RUnlock()
548567

549568
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
@@ -94,6 +94,31 @@ __thread php_stream *worker_signaling_stream = NULL;
9494
__thread char *captured_last_php_error = NULL;
9595
__thread HashTable *sandboxed_env = NULL;
9696

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

988-
char *error = go_frankenphp_get_vars(thread_index, (char *)ZSTR_VAL(name),
989-
ZSTR_LEN(name), return_value);
1013+
/* Look up a cached entry first so Go can short-circuit the copy when
1014+
* the background worker has not changed its vars since we last read
1015+
* them. On a cache hit we reuse the same zval, so $vars === $prev_vars
1016+
* holds across repeated reads within one request. */
1017+
uint64_t caller_version = 0;
1018+
uint64_t out_version = 0;
1019+
bg_vars_cache_entry *cached = NULL;
1020+
if (bg_vars_cache) {
1021+
zval *entry_zv = zend_hash_find(bg_vars_cache, name);
1022+
if (entry_zv) {
1023+
cached = Z_PTR_P(entry_zv);
1024+
caller_version = cached->version;
1025+
}
1026+
}
1027+
1028+
char *error = go_frankenphp_get_vars(
1029+
thread_index, (char *)ZSTR_VAL(name), ZSTR_LEN(name), return_value,
1030+
cached ? &caller_version : NULL, &out_version);
9901031
if (error) {
9911032
zend_throw_exception(spl_ce_RuntimeException, error, 0);
9921033
free(error);
9931034
RETURN_THROWS();
9941035
}
1036+
1037+
if (cached && out_version == caller_version) {
1038+
/* Cache hit: Go skipped the copy. Return the cached zval. */
1039+
ZVAL_COPY(return_value, &cached->value);
1040+
return;
1041+
}
1042+
1043+
/* Cache miss: store the fresh copy so subsequent reads within this
1044+
* request can be served without walking persistent memory. */
1045+
if (!bg_vars_cache) {
1046+
bg_vars_cache = malloc(sizeof(HashTable));
1047+
zend_hash_init(bg_vars_cache, 4, NULL, bg_vars_cache_dtor, 1);
1048+
}
1049+
bg_vars_cache_entry *entry = malloc(sizeof(*entry));
1050+
entry->version = out_version;
1051+
ZVAL_COPY(&entry->value, return_value);
1052+
zval entry_zv;
1053+
ZVAL_PTR(&entry_zv, entry);
1054+
zend_hash_update(bg_vars_cache, name, &entry_zv);
9951055
}
9961056

9971057
PHP_FUNCTION(frankenphp_ensure_background_worker) {
@@ -1583,6 +1643,11 @@ static void *php_thread(void *arg) {
15831643
* so background-worker boot failures can surface the cause. */
15841644
frankenphp_capture_last_php_error();
15851645

1646+
/* Invalidate the per-request get_vars cache before php_request_shutdown
1647+
* tears down request memory: the cached zvals live in request memory
1648+
* and can't be freed after shutdown runs. */
1649+
bg_vars_cache_reset();
1650+
15861651
/* shutdown the request, potential bailout to zend_catch */
15871652
php_request_shutdown((void *)0);
15881653
frankenphp_free_request_context();
@@ -1602,6 +1667,7 @@ static void *php_thread(void *arg) {
16021667
if (!has_attempted_shutdown) {
16031668
/* php_request_shutdown() was not called, force a shutdown now */
16041669
reset_sandboxed_environment();
1670+
bg_vars_cache_reset();
16051671
zend_try { php_request_shutdown((void *)0); }
16061672
zend_catch {}
16071673
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)