]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: stick-table: make stktable_get_entry() look up under a read lock
authorWilly Tarreau <w@1wt.eu>
Tue, 11 Oct 2022 13:22:42 +0000 (15:22 +0200)
committerWilly Tarreau <w@1wt.eu>
Wed, 12 Oct 2022 12:19:05 +0000 (14:19 +0200)
On a 24-core machine doing lots of track-sc, it was found that the lock
in stktable_get_entry() was responsible for 25% of the CPU alone. It's
sad because most of its job is to protect the table during the lookup.

Here we're taking a slightly different approach: the lock is first taken
for reads during the lookup, and only in case of failure we switch it for
a write lock. We don't even perform an upgrade here since an allocation
is needed between the two, it would be wasted to do it under the lock,
and is generally not a good idea, so better release the read lock and
try again.

Here the performance under 48 threads with 3 trackers on the same table
jumped from 455k to 2.07M, or 4.55x! Note that the same approach should
be possible for stktable_set_entry().

src/stick_table.c

index 44a115fd1ac58988876f98b970e9fb1d7f85c9b6..7bb3b239320d05c420d82b010debea7a6c674cb8 100644 (file)
@@ -519,45 +519,44 @@ struct stksess *__stktable_store(struct stktable *t, struct stksess *ts)
 
 /* Returns a valid or initialized stksess for the specified stktable_key in the
  * specified table, or NULL if the key was NULL, or if no entry was found nor
- * could be created. The entry's expiration is updated.
+ * could be created. The entry's expiration is updated. This function locks the
+ * table, and the refcount of the entry is increased.
  */
-struct stksess *__stktable_get_entry(struct stktable *table, struct stktable_key *key)
+struct stksess *stktable_get_entry(struct stktable *table, struct stktable_key *key)
 {
        struct stksess *ts, *ts2;
 
        if (!key)
                return NULL;
 
-       ts = __stktable_lookup_key(table, key);
-       if (ts == NULL) {
-               /* entry does not exist, initialize a new one */
-               ts = __stksess_new(table, key);
-               if (!ts)
-                       return NULL;
-               ts2 = __stktable_store(table, ts);
-               if (unlikely(ts2 != ts)) {
-                       /* another entry was added in the mean time, let's
-                        * switch to it.
-                        */
-                       __stksess_free(table, ts);
-                       ts = ts2;
-               }
+       ts = stktable_lookup_key(table, key);
+       if (ts)
+               return ts;
+
+       /* No such entry exists, let's try to create a new one. For this we'll
+        * need an exclusive access. We don't need an atomic upgrade, this is
+        * rare and an unlock+lock sequence will do the job fine. Given that
+        * this will not be atomic, the missing entry might appear in the mean
+        * tome so we have to be careful that the one we try to insert is the
+        * one we find.
+        */
+       HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &table->lock);
+
+       ts = __stksess_new(table, key);
+       if (!ts)
+               goto end;
+
+       ts2 = __stktable_store(table, ts);
+       if (unlikely(ts2 != ts)) {
+               /* another entry was added in the mean time, let's
+                * switch to it.
+                */
+               __stksess_free(table, ts);
+               ts = ts2;
        }
-       return ts;
-}
-/* Returns a valid or initialized stksess for the specified stktable_key in the
- * specified table, or NULL if the key was NULL, or if no entry was found nor
- * could be created. The entry's expiration is updated.
- * This function locks the table, and the refcount of the entry is increased.
- */
-struct stksess *stktable_get_entry(struct stktable *table, struct stktable_key *key)
-{
-       struct stksess *ts;
 
-       HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &table->lock);
-       ts = __stktable_get_entry(table, key);
-       if (ts)
-               ts->ref_cnt++;
+       HA_ATOMIC_INC(&ts->ref_cnt);
+ end:
        HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &table->lock);
 
        return ts;