From 7538b4d719c8c0c14fd361256056f80fa1908d10 Mon Sep 17 00:00:00 2001
From: Tobias Bengtsson <me@tobiasbengtsson.se>
Date: Thu, 9 Nov 2023 20:29:19 +0100
Subject: [PATCH 1/2] Make APCng updateGauge with set command lock-free in
 critical path

When setting a value on a gauge, first apcu_store is called in
updateGauge and then apcu_add is called in storeMetadata. Both of
these operations takes a write lock in APCu which can cause contention.

- Change updateGauge to use a compare-and-swap algorithm instead of
apcu_store, unless we're adding something for the first time(s) in
which case we just set it with apcu_store.
- Don't call updateMetadata and storeLabelKeys unless it's the first
time we're setting a gauge (matches the behaviour of inc/dec).
- For extra safety, make a cheap rlock-only check in storeMetadata if
the key exists before calling apcu_add with a wlock.

Signed-off-by: Tobias Bengtsson <me@tobiasbengtsson.se>
---
 src/Prometheus/Storage/APCng.php | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/Prometheus/Storage/APCng.php b/src/Prometheus/Storage/APCng.php
index 6628ef97..5448fc95 100644
--- a/src/Prometheus/Storage/APCng.php
+++ b/src/Prometheus/Storage/APCng.php
@@ -169,16 +169,34 @@ public function updateSummary(array $data): void
     public function updateGauge(array $data): void
     {
         $valueKey = $this->valueKey($data);
+        $old = apcu_fetch($valueKey);
         if ($data['command'] === Adapter::COMMAND_SET) {
-            apcu_store($valueKey, $this->convertToIncrementalInteger($data['value']), 0);
-            $this->storeMetadata($data);
-            $this->storeLabelKeys($data);
+            $new = $this->convertToIncrementalInteger($data['value']);
+            if ($old === false) {
+                apcu_store($valueKey, $new, 0);
+                $this->storeMetadata($data);
+                $this->storeLabelKeys($data);
+
+                return;
+            }
+
+            for ($loops = 0; $loops < self::MAX_LOOPS; $loops++) {
+                if (apcu_cas($valueKey, $old, $new)) {
+                    break;
+                }
+                $old = apcu_fetch($valueKey);
+                if ($old === false) {
+                    apcu_store($valueKey, $new, 0);
+                    $this->storeMetadata($data);
+                    $this->storeLabelKeys($data);
+
+                    return;
+                }
+            }
 
             return;
         }
 
-        $old = apcu_fetch($valueKey);
-
         if ($old === false) {
             apcu_add($valueKey, 0, 0);
             $this->storeMetadata($data);
@@ -896,6 +914,10 @@ private function decodeLabelKey(string $str): string
     private function storeMetadata(array $data, bool $encoded = true): void
     {
         $metaKey = $this->metaKey($data);
+        if (apcu_exists($metaKey)) {
+            return;
+        }
+
         $metaData = $this->metaData($data);
         $toStore = $metaData;
 

From c488171a016c74820fd40b9df105606ca3509e01 Mon Sep 17 00:00:00 2001
From: Tobias Bengtsson <me@tobiasbengtsson.se>
Date: Thu, 9 Nov 2023 20:58:14 +0100
Subject: [PATCH 2/2] Make APC updateGauge with set command lock-free in
 critical path

When setting a value on a gauge, apcu_store is always called two
times, taking a write lock each that can cause contention.

- Replace apcu_store with compare-and-swap algorithm unless we store
for the first time(s)
- Eagerly fetch old value instead of apcu_exists, since we will anyway
need it for the compare-and-swap algorithms that follows, saving one
call to APCu
- Make sure we never enter an infinite loop in the compare-and-swap
section by falling back to a first-time insert if $old === false. A
fix for the potential infinite loop below will be done in a separate
commit.

Signed-off-by: Tobias Bengtsson <me@tobiasbengtsson.se>
---
 src/Prometheus/Storage/APC.php | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/src/Prometheus/Storage/APC.php b/src/Prometheus/Storage/APC.php
index c45f1940..c8a3933f 100644
--- a/src/Prometheus/Storage/APC.php
+++ b/src/Prometheus/Storage/APC.php
@@ -123,11 +123,32 @@ public function updateSummary(array $data): void
     public function updateGauge(array $data): void
     {
         $valueKey = $this->valueKey($data);
+        $old = apcu_fetch($valueKey);
         if ($data['command'] === Adapter::COMMAND_SET) {
-            apcu_store($valueKey, $this->toBinaryRepresentationAsInteger($data['value']));
-            apcu_store($this->metaKey($data), json_encode($this->metaData($data)));
+            $new = $this->toBinaryRepresentationAsInteger($data['value']);
+            if ($old === false) {
+                apcu_store($valueKey, $new);
+                apcu_store($this->metaKey($data), json_encode($this->metaData($data)));
+                return;
+            } else {
+                // Taken from https://github.com/prometheus/client_golang/blob/66058aac3a83021948e5fb12f1f408ff556b9037/prometheus/value.go#L91
+                while (true) {
+                    if ($old !== false) {
+                        if (apcu_cas($valueKey, $old, $new)) {
+                            return;
+                        } else {
+                            $old = apcu_fetch($valueKey);
+                        }
+                    } else {
+                        // Cache got evicted under our feet? Just consider it a fresh/new insert and move on.
+                        apcu_store($valueKey, $new);
+                        apcu_store($this->metaKey($data), json_encode($this->metaData($data)));
+                        return;
+                    }
+                }
+            }
         } else {
-            if (!apcu_exists($valueKey)) {
+            if ($old === false) {
                 $new = apcu_add($valueKey, $this->toBinaryRepresentationAsInteger(0));
                 if ($new) {
                     apcu_store($this->metaKey($data), json_encode($this->metaData($data)));