fix: fix LRU impl and more

Author: zeriyoshi

ext/user_cache/tests/user_cache_entries_hint.phpt

@@ -24,11 +24,14 @@ echo shell_exec("$php $args -d user_cache.entries_hint=-1 -r " . escapeshellarg(
 $out = shell_exec("$php $args -d user_cache.entries_hint=16777213 -r " . escapeshellarg($code) . " 2>&1");
 echo $out, "\n";
 
-/* The clamped table plus its per-key lock region (1024 records * 40 bytes
- * at this scale) must still leave half the segment for value data. */
+/* The clamped table plus its per-key lock region (1024 records at this
+ * scale) must still leave half the segment for value data. Entry records
+ * are 48 bytes and lock records 40 on 64-bit layouts, 44 and 36 on 32-bit;
+ * each entry also carries a 4-byte access stamp. */
+[$entry_bytes, $lock_bytes] = PHP_INT_SIZE >= 8 ? [48, 40] : [44, 36];
 preg_match('/(\d+)\s*$/', $out, $m);
 $cap = (int) $m[1];
-var_dump($cap >= 100000, $cap * (48 + 4) + 1024 * 40 <= 8 * 1024 * 1024);
+var_dump($cap >= 100000, $cap * ($entry_bytes + 4) + 1024 * $lock_bytes <= 8 * 1024 * 1024);
 ?>
 --EXPECTF--
 int(1361)

ext/user_cache/tests/user_cache_eviction_policies.phpt

@@ -9,7 +9,11 @@ user_cache.shm_size=4M
 <?php
 $php = escapeshellarg(getenv('TEST_PHP_EXECUTABLE') ?: PHP_BINARY);
 $args = '-n -d user_cache.enable=1 -d user_cache.enable_cli=1 -d user_cache.shm_size=4M';
-$code = <<<'CODE'
+/* Run the child from a file: escapeshellarg() on Windows strips the double
+ * quotes and percent signs the inline code would need. */
+$child = __DIR__ . '/user_cache_eviction_policies_child.php';
+file_put_contents($child, <<<'CODE'
+<?php
 $cache = UserCache\Cache::getPool('p');
 $blob = str_repeat('x', 8192);
 $ok = 0;
@@ -27,16 +31,19 @@ printf(
     (int) ($status->getExpungeCount() > 0),
     (int) ($status->getStoreFailureCount() > 0)
 );
-CODE;
+CODE);
 
 foreach (['lru', 'clear', 'none'] as $policy) {
     echo $policy, ': ', shell_exec(
-        "$php $args -d user_cache.eviction_policy=$policy -r " . escapeshellarg($code)
+        "$php $args -d user_cache.eviction_policy=$policy " . escapeshellarg($child)
     );
 }
 
 /* Invalid values are rejected at INI time and fall back to the lru default. */
-echo shell_exec("$php $args -d user_cache.eviction_policy=bogus -r " . escapeshellarg('echo "rejected\n";') . " 2>&1");
+echo shell_exec("$php $args -d user_cache.eviction_policy=bogus -r " . escapeshellarg("echo 'rejected';") . " 2>&1"), "\n";
+
+/* Unlink here instead of --CLEAN-- so --repeat runs keep this test. */
+unlink($child);
 ?>
 --EXPECTF--
 lru: stored=600 full=1 evicted=1 wiped=0 failed=0

ext/user_cache/tests/user_cache_eviction_size_class.phpt

@@ -0,0 +1,40 @@
+--TEST--
+UserCache\Cache: LRU eviction absorbs a block size-class step without wiping
+--INI--
+user_cache.enable=1
+user_cache.enable_cli=1
+opcache.file_cache_only=0
+user_cache.shm_size=4M
+--FILE--
+<?php
+$cache = UserCache\Cache::getPool('sizeclass');
+
+/* Fill the segment with one value size class. */
+$small = str_repeat('a', 8192);
+for ($i = 0; $i < 600; $i++) {
+    $cache->store('s' . $i, $small);
+}
+
+/* Churn with values two alignment steps larger: every LRU victim frees a
+ * block strictly smaller than the incoming store needs, so eviction must
+ * keep going until a fitting region exists instead of falling back to a
+ * full wipe. */
+$large = str_repeat('b', 8192 + 16);
+$ok = 0;
+for ($i = 0; $i < 300; $i++) {
+    if ($cache->store('l' . $i, $large)) {
+        $ok++;
+    }
+}
+
+$status = UserCache\Cache::getStatus();
+var_dump($ok === 300);
+var_dump($status->getEvictionCount() > 0);
+var_dump($status->getExpungeCount() === 0);
+var_dump($cache->fetch('l299') === $large);
+?>
+--EXPECT--
+bool(true)
+bool(true)
+bool(true)
+bool(true)

ext/user_cache/tests/user_cache_remember_by_ref_callback.phpt

@@ -9,6 +9,10 @@ user_cache.shm_size=16M
 <?php
 $cache = UserCache\Cache::getPool('remember-by-ref');
 
+/* Drop leftovers so a --repeat re-run in the same process misses again. */
+$cache->delete('by-ref');
+$cache->delete('plain');
+
 $g = 42;
 $byRef = function &(string $key) use (&$g) {
     return $g;

ext/user_cache/user_cache_alloc_posix.c

@@ -41,8 +41,8 @@ static int user_cache_alloc_posix_create_segments(size_t requested_size, php_use
 	 * result as a signed int: its -1 error return in a size_t would pass
 	 * the > 0 guard and index far outside the array (see the equivalent
 	 * upstream fix in ext/opcache/shared_alloc_posix.c, GH-22429). */
+	size_t shared_segment_lg_index = 0, shared_segment_sindexes[3] = {0};
 	const size_t entries = sizeof(shared_segment_sindexes) / sizeof(shared_segment_sindexes[0]);
-	 size_t shared_segment_lg_index = 0, shared_segment_sindexes[3] = {0};
 	int i, shared_segment_sizes;
 
 	shared_segment_sizes = getpagesizes(shared_segment_sindexes, entries);
@@ -52,6 +52,7 @@ static int user_cache_alloc_posix_create_segments(size_t requested_size, php_use
 			if (shared_segment_sindexes[i] != 0 &&
 			    !(requested_size % shared_segment_sindexes[i])) {
 				shared_segment_lg_index = i;
+
 				break;
 			}
 		}
@@ -62,6 +63,7 @@ static int user_cache_alloc_posix_create_segments(size_t requested_size, php_use
 	*shared_segments_p = (php_user_cache_shm_segment_posix **) calloc(1, sizeof(php_user_cache_shm_segment_posix) + sizeof(void *));
 	if (!*shared_segments_p) {
 		*error_in = "calloc";
+
 		return PHP_USER_CACHE_ALLOC_FAILURE;
 	}
 	shared_segment = (php_user_cache_shm_segment_posix *)((char *)(*shared_segments_p) + sizeof(void *));

ext/user_cache/user_cache_entries.c

@@ -2388,32 +2388,14 @@ static bool user_cache_publish_prepared_shared_graph_locked(
 	);
 }
 
-static size_t user_cache_entry_storage_footprint_locked(const php_user_cache_entry *entry)
-{
-	size_t footprint = 0;
-
-	if (entry->value_offset != 0 && user_cache_value_uses_offset(entry->value_type)) {
-		footprint += php_user_cache_block_payload_capacity(entry->value_offset);
-	}
-
-	if (entry->key_offset != 0 &&
-		(entry->reserved & PHP_USER_CACHE_ENTRY_RESERVED_COMBINED_VALUE_KEY) == 0
-	) {
-		footprint += php_user_cache_block_payload_capacity(entry->key_offset);
-	}
-
-	return footprint;
-}
-
-static bool user_cache_evict_lru_locked(size_t needed_size)
+static bool user_cache_evict_lru_locked(size_t needed_size, size_t needed_key_size)
 {
 	php_user_cache_header *header = php_user_cache_header_ptr();
 	php_user_cache_entry *entries, *entry;
 	uint64_t lock_now, lock_now_rel;
 	uint32_t hand, slot, scanned, collected, stamp, best_slot,
 		best_stamp = 0, victims = 0, *stamps
 	;
-	size_t freed = 0;
 	int8_t graph_quiescent = -1;
 	bool evicted_any = false;
 
@@ -2425,7 +2407,6 @@ static bool user_cache_evict_lru_locked(size_t needed_size)
 	stamps = php_user_cache_access_stamps_ptr(header);
 	hand = header->eviction_hand % header->capacity;
 	lock_now = (uint64_t) time(NULL);
-	/* The lock probe compares time_base-relative lease deadlines. */
 	lock_now_rel = php_user_cache_time_rel(header, lock_now);
 
 	user_cache_access_note_time(lock_now);
@@ -2455,9 +2436,6 @@ static bool user_cache_evict_lru_locked(size_t needed_size)
 					continue;
 				}
 
-				/* An undrained reader set would orphan the payload instead
-				 * of freeing it, so its bytes must not count toward freed;
-				 * probe lazily so graph-free scans never pay the drain wait. */
 				if (graph_quiescent < 0) {
 					graph_quiescent = php_user_cache_quiesce_graph_payloads_locked() ? 1 : 0;
 				}
@@ -2467,10 +2445,6 @@ static bool user_cache_evict_lru_locked(size_t needed_size)
 				}
 			}
 
-			/* Both skips are load-bearing: evicting a per-key-locked entry
-			 * would break the writer serialization contract, and counting a
-			 * pinned graph payload as freed makes the retry fail into the
-			 * full-clear fallback. */
 			if (php_user_cache_entry_key_lock_active_locked(
 					header,
 					entry->hash,
@@ -2496,15 +2470,16 @@ static bool user_cache_evict_lru_locked(size_t needed_size)
 		}
 
 		entry = &entries[best_slot];
-		freed += user_cache_entry_storage_footprint_locked(entry);
 
 		user_cache_delete_entry_locked(header, entry);
 
 		header->eviction_count++;
 		victims++;
 		evicted_any = true;
 
-		if (needed_size == 0 || freed >= needed_size) {
+		if (needed_size == 0 ||
+			php_user_cache_alloc_can_satisfy_locked(needed_size, needed_key_size)
+		) {
 			break;
 		}
 	}
@@ -2517,6 +2492,7 @@ static bool user_cache_evict_lru_locked(size_t needed_size)
 static bool user_cache_reclaim_space_for_store_locked(
 		bool can_reclaim,
 		size_t needed_size,
+		size_t needed_key_size,
 		bool *expired_retry_used,
 		bool *evict_retry_used,
 		bool *clear_retry_used)
@@ -2545,7 +2521,7 @@ static bool user_cache_reclaim_space_for_store_locked(
 	) {
 		*evict_retry_used = true;
 
-		if (user_cache_evict_lru_locked(needed_size)) {
+		if (user_cache_evict_lru_locked(needed_size, needed_key_size)) {
 			return true;
 		}
 	}
@@ -2622,6 +2598,7 @@ static php_user_cache_store_attempt_result user_cache_store_attempt_locked(
 			user_cache_reclaim_space_for_store_locked(
 				true,
 				0,
+				0,
 				expired_retry_used,
 				evict_retry_used,
 				clear_retry_used
@@ -2833,6 +2810,7 @@ static php_user_cache_store_attempt_result user_cache_store_attempt_locked(
 					user_cache_reclaim_space_for_store_locked(
 						true,
 						prepared->payload_size + key_size,
+						0,
 						expired_retry_used,
 						evict_retry_used,
 						clear_retry_used
@@ -2878,6 +2856,7 @@ static php_user_cache_store_attempt_result user_cache_store_attempt_locked(
 					user_cache_reclaim_space_for_store_locked(
 						true,
 						prepared->payload_size,
+						0,
 						expired_retry_used,
 						evict_retry_used,
 						clear_retry_used
@@ -2969,7 +2948,14 @@ static php_user_cache_store_attempt_result user_cache_store_attempt_locked(
 	if (options->retry_after_memory_pressure &&
 		user_cache_reclaim_space_for_store_locked(
 			can_reclaim,
-			prepared->payload_size + key_size,
+			use_combined_publish
+				? prepared->payload_size + key_size
+				: prepared->payload_size
+			,
+			use_combined_publish
+				? 0
+				: key_size
+			,
 			expired_retry_used,
 			evict_retry_used,
 			clear_retry_used

ext/user_cache/user_cache_internal.h

@@ -764,6 +764,7 @@ bool php_user_cache_header_init_locked(void);
 bool php_user_cache_header_adoptable_locked(void);
 void php_user_cache_free_locked(uint32_t payload_offset);
 uint32_t php_user_cache_alloc_locked(size_t size, const void *src);
+bool php_user_cache_alloc_can_satisfy_locked(size_t size, size_t key_size);
 bool php_user_cache_startup_storage_before_request(void);
 void php_user_cache_shutdown_storage(void);
 void php_user_cache_ensure_ready_impl(void);

ext/user_cache/user_cache_storage.c

@@ -4193,6 +4193,55 @@ uint32_t php_user_cache_alloc_locked(size_t size, const void *src)
 	return user_cache_alloc_from_tail_locked(header, size, total_size, src);
 }
 
+bool php_user_cache_alloc_can_satisfy_locked(size_t size, size_t key_size)
+{
+	php_user_cache_header *header = php_user_cache_header_ptr();
+	uint32_t free_offset;
+	size_t value_total, key_total, region, largest, second = 0;
+
+	if (!header || size == 0 || size > UINT32_MAX - sizeof(php_user_cache_block)) {
+		return false;
+	}
+
+	value_total = PHP_USER_CACHE_ALIGNED_SIZE(sizeof(php_user_cache_block) + size);
+	key_total = key_size != 0
+		? PHP_USER_CACHE_ALIGNED_SIZE(sizeof(php_user_cache_block) + key_size)
+		: 0
+	;
+	if (value_total > UINT32_MAX || key_total > UINT32_MAX - value_total) {
+		return false;
+	}
+
+	region = header->next_free <= header->data_size
+		? header->data_size - header->next_free
+		: 0
+	;
+	largest = region;
+
+	for (free_offset = header->free_list;
+		free_offset != 0;
+		free_offset = php_user_cache_block_ptr(free_offset)->next_free
+	) {
+		region = php_user_cache_block_ptr(free_offset)->size;
+
+		if (region >= value_total + key_total) {
+			return true;
+		}
+
+		if (region > largest) {
+			second = largest;
+			largest = region;
+		} else if (region > second) {
+			second = region;
+		}
+	}
+
+
+	return largest >= value_total + key_total ||
+		(largest >= value_total && (key_total == 0 || second >= key_total))
+	;
+}
+
 bool php_user_cache_startup_storage_before_request(void)
 {
 	php_user_cache_storage *storage = &php_user_cache_active_context()->storage;