Skip to content

Rewrite promise implementation - #97

Merged
jgraichen merged 2 commits into
mainfrom
chore/optimize-promise
Sep 27, 2026
Merged

jgraichen merged 2 commits into
mainfrom
chore/optimize-promise

Conversation

@jgraichen

Copy link
Copy Markdown
Owner

This is a large rewrite of the promise implementation, replacing the comparable slow Concurrent::IVar with a lightweight custom implementation and specifying a few edge cases.

  1. IVar used several Mutex and events internally and synchronized more than necessary. The new implementation uses a single Mutex, reducing contention and improving performance.

  2. Do not reject a promise on a timeout. Waiting on a promise with a timeout that expires will not change the promise's state, allowing subsequent waits to continue where the last one left off. Because the previous simple implementation was driven by exceptions, every exception, including a timeout, rejected the promise. Now, it is possible to wait with different timeouts.

  3. Honor the timeout of all threads waiting on the same promise, not only of the thread running its #then blocks.

    Before, all waiting happened on the Mutex synchronization, but that does not have a timeout at all. A long-running then-block being run on one thread could block other threads waiting on the same promise even if they had short timeouts.

    The new implementation separates between running and claiming/waiting to run, so threads with short timeouts are not blocked. Furthermore, because a timeout no longer affects the state, longer timeouts can still drive a promise to completion.

Technically, the new promise implementation is no longer based on Concurrent::IVar and does not inherit from it anymore. A promise could have been used in other concurrent-ruby contexts or any other Concurrent::IVar as a dependency. This is no longer possible!

Lately, these changes reduce contention and improve the overall performance quite a bit. Additionally, a lot fewer objects are allocated.

Which methods shall become public or not is still under consideration.

Remove usage of `Concurrent::Event` in specs and replace it with `Queue`
so that this spec no longer depends on concurrent-ruby.
This is a large rewrite of the promise implementation, replacing the
comparable slow `Concurrent::IVar` with a lightweight custom
implementation, and specifying a few edge cases.

1. `IVar` used several `Mutex` and events internally, and synchronized
   more than necessary. The new implementation uses a single `Mutex`,
   reducing contention and improving performance.

2. Do not reject a promise on a timeout. Waiting on a promise with a
   timeout that expires will not change the promise's state, allowing
   subsequent waits to continue where the last one left off.

   Because the previous simple implementation was driven by exceptions,
   every exception, including a timeout, rejected the promise.

   Now, it is possible to wait with different timeouts.

3. Honor the timeout of all threads waiting on the same promise, not
   only of the thread running its `#then` blocks.

   Before, all waiting happened on the `Mutex` synchronization, but that
   does not have a timeout at all. A long-running then-block being run
   on one thread could block other threads waiting on the same promise
   even if they had short timeouts.

   The new implementation separates between running and claiming/waiting
   to run, so threads with short timeouts are not blocked. Furthermore,
   because a timeout no longer affects the state, longer timeouts can
   still drive a promise to completion.

Technically, the new promise implementation is no longer based on
`Concurrent::IVar` and does not inherit from it anymore. A promise could
have been used in other concurrent-ruby contexts, or any other
`Concurrent::IVar` as a dependency. This is no longer possible!

Lately, these changes reduce contention and improve the overall
performance quite a bit. Additionally, a lot less objects are allocated.

Which methods shall become public or not is still under consideration.
@jgraichen jgraichen self-assigned this Sep 27, 2026
@jgraichen
jgraichen merged commit 77e01ca into main Sep 27, 2026
22 checks passed
@codecov

codecov Bot commented Sep 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.07407% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.35%. Comparing base (c5e11ee) to head (cfeecdf).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
lib/restify/promise.rb 98.87% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
+ Coverage   97.23%   97.35%   +0.11%     
==========================================
  Files          28       28              
  Lines        1121     1171      +50     
==========================================
+ Hits         1090     1140      +50     
  Misses         31       31              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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