Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Allow middleware thread-safe without duping #6

Merged
merged 1 commit into from
Dec 2, 2024
Merged

Conversation

liveh2o
Copy link
Owner

@liveh2o liveh2o commented Dec 1, 2024

The initial Allow middleware implementation was not thread-safe. A "temporary" (eight years ago!) hack was put in place that simply duped the middleware stack to ensure threadsafety. Not only is this a hack, but it also adds unnecessary overhead to each request since each dup generated more objects that would need to be GC'd.

This change makes the Allow middleware thread-safe without duping, and yields a whopping 85x (!) increase in performance (via bin/benchmark):

Before:

ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [arm64-darwin22]
Warming up --------------------------------------
    Hit    2.318k i/100ms
    Miss   2.317k i/100ms
Calculating -------------------------------------
    Hit   22.267k (± 2.8%) i/s (44.91 μs/i) - 111.264k in 5.000717s
    Miss  22.266k (± 1.2%) i/s (44.91 μs/i) - 113.533k in 5.099622s

After:

ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [arm64-darwin22]
Warming up --------------------------------------
    Hit  188.921k i/100ms
    Miss 186.268k i/100ms
Calculating -------------------------------------
    Hit    1.859M (± 0.5%) i/s (537.92 ns/i) - 9.446M in 5.081332s
    Miss   1.860M (± 1.1%) i/s (537.70 ns/i) - 9.313M in 5.008448s

This implementation uses a simplified Config class that is no longer backed by a hash. This required dropping support for passing config options to the middleware, but that likely was mostly used in tests.

@liveh2o liveh2o force-pushed the ah/optimize branch 4 times, most recently from 1c7904a to 80e0b03 Compare December 2, 2024 17:27
@liveh2o liveh2o marked this pull request as ready for review December 2, 2024 17:27
The initial Allow middleware was not thread-safe. A "temporary" (eight
years ago!) hack was put in place that simply duped the middleware stack
to ensure thread-safety. Not only is this a hack, but it also adds
unnecessary overhead to each request since each dup generated more
objects that would need to be GC'd.

This change makes the Allow middleware thread-safe without duping, and
yields a whopping 85x (!) increase in performance (via bin/benchmark):

Before:

    ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [arm64-darwin22]
    Warming up -------------qq-------------------------
        Hit    2.318k i/100ms
        Miss   2.317k i/100ms
    Calculating -------------------------------------
        Hit   22.267k (± 2.8%) i/s (44.91 μs/i) - 111.264k in 5.000717s
        Miss  22.266k (± 1.2%) i/s (44.91 μs/i) - 113.533k in 5.099622s

After:

    ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [arm64-darwin22]
    Warming up --------------------------------------
        Hit  188.921k i/100ms
        Miss 186.268k i/100ms
    Calculating -------------------------------------
        Hit    1.859M (± 0.5%) i/s (537.92 ns/i) - 9.446M in 5.081332s
        Miss   1.860M (± 1.1%) i/s (537.70 ns/i) - 9.313M in 5.008448s

This implementation uses a simplified Config class that is no longer
backed by a hash. This required dropping support for passing config
options to the middleware, but that likely was mostly used in tests.
@liveh2o liveh2o merged commit 393da17 into main Dec 2, 2024
5 checks passed
@liveh2o liveh2o deleted the ah/optimize branch December 2, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant