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

Add monitor API back #317

Merged
merged 2 commits into from
Jan 8, 2025
Merged

Add monitor API back #317

merged 2 commits into from
Jan 8, 2025

Conversation

slawlor
Copy link
Owner

@slawlor slawlor commented Jan 8, 2025

This PR re-adds the monitor API, but with an option in the map to save allocations when unused.

Additionally we move to Option types to save on allocations for regular supervisions (if the actor doesn't supervise anything, we don't need to create the empty map)

This was originally removed in #260 to save on memory, but it was used by the community and therefore a valuable API as noted in #307

TBD mem measurement

…ions when unused.

Additionally move to Option<HashMap> types to save on allocations for regular supervisions (if the actor doesn't supervise anything, we don't need to create the empty map)
@slawlor
Copy link
Owner Author

slawlor commented Jan 8, 2025

Benchmarking maximum RSS with $ /usr/bin/time -l cargo run --example a_whole_lotta --release

Branch main

        2.99 real         2.65 user         2.20 sys
          1273331712  maximum resident set size
                   0  average shared memory size
                   0  average unshared data size
                   0  average unshared stack size
               84109  page reclaims
                   0  page faults
                   0  swaps
                   0  block input operations
                   0  block output operations
                   0  messages sent
                   0  messages received
                   0  signals received
               10335  voluntary context switches
              312179  involuntary context switches
         29473427138  instructions retired
         14513235212  cycles elapsed
          1272550144  peak memory footprint

This PR

        3.16 real         2.67 user         2.16 sys
          1284161536  maximum resident set size
                   0  average shared memory size
                   0  average unshared data size
                   0  average unshared stack size
               84688  page reclaims
                   0  page faults
                   0  swaps
                   0  block input operations
                   0  block output operations
                   0  messages sent
                   0  messages received
                   0  signals received
               10037  voluntary context switches
              309936  involuntary context switches
         29886600296  instructions retired
         14417672946  cycles elapsed
          1282510976  peak memory footprint

Which is a regression of ~1kb/actor. This is enough I think to warrant gating the functionality behind a feature for opt-in usage if you want the monitor API.

Copy link

codecov bot commented Jan 8, 2025

Codecov Report

Attention: Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.08%. Comparing base (0f0483f) to head (59b4af4).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
ractor/src/actor/supervision.rs 95.45% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #317      +/-   ##
==========================================
+ Coverage   82.07%   82.08%   +0.01%     
==========================================
  Files          61       61              
  Lines       12183    12198      +15     
==========================================
+ Hits         9999    10013      +14     
- Misses       2184     2185       +1     

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

…age on monitors, and update API and README docs
@slawlor slawlor marked this pull request as ready for review January 8, 2025 15:29
@slawlor slawlor merged commit b0cd1a0 into main Jan 8, 2025
15 checks passed
@slawlor slawlor deleted the monitor branch January 8, 2025 15:35
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