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

Traverse without recursion #2168

Merged
merged 15 commits into from
Nov 15, 2024
Merged

Traverse without recursion #2168

merged 15 commits into from
Nov 15, 2024

Conversation

RobinTail
Copy link
Owner

@RobinTail RobinTail commented Nov 14, 2024

Avoiding recursion should decrease memory consumption by replacing the utilization of call stack to a stack of entities to process

https://www.nearform.com/insights/tracking-memory-allocation-node-js/

   ✓ Experiment for routing walker (2) 1247ms
     name             hz     min     max    mean     p75     p99    p995    p999     rme  samples
   · current    3,691.20  0.2053  2.7257  0.2709  0.2885  0.3817  0.4519  2.1826  ±1.49%     1846
   · featured  50,253.26  0.0123  5.8032  0.0199  0.0165  0.0423  0.1116  2.7895  ±7.10%    25127   fastest

Turned out that the performance is mostly affected by assert(..., new Error()) pattern, partially introduced by #1371

@RobinTail RobinTail added the refactoring The better way to achieve the same result label Nov 14, 2024
@RobinTail RobinTail added this to the v21 milestone Nov 14, 2024
@RobinTail RobinTail changed the base branch from master to make-v21 November 14, 2024 21:46
Copy link

coveralls-official bot commented Nov 14, 2024

Coverage Status

coverage: 100.0%. remained the same
when pulling 36608df on traverse-no-recursion
into b8f0906 on make-v21.

@RobinTail RobinTail changed the title Exp: Traverse without recursion Traverse without recursion Nov 15, 2024
RobinTail added a commit that referenced this pull request Nov 15, 2024
partially reverts #1371
for performance reasons found in #2168
@RobinTail RobinTail mentioned this pull request Nov 15, 2024
@RobinTail
Copy link
Owner Author

After fixing in #2169

   ✓ Experiment for routing walker (2) 1269ms
     name             hz     min      max    mean     p75     p99    p995    p999     rme  samples
   · old       46,289.59  0.0114   5.7957  0.0216  0.0179  0.0497  0.1211  2.8412  ±6.92%    23145
   · featured  48,947.95  0.0132  10.0317  0.0204  0.0168  0.0273  0.0357  3.1846  ±8.16%    24474   fastest

@RobinTail
Copy link
Owner Author

RobinTail commented Nov 15, 2024

Evaluation using --trace-gc in Node 22 with a help of ChatGPT

tsx runner involved in both cases

Key Observations

  1. Scavenge Performance:

    • Old Logs: Scavenge cycles often exceed 1 ms (e.g., 1.19 ms, 1.33 ms) with notable occurrences of allocation failures leading to fallback GC strategies.
    • New Logs: Scavenge cycles are generally faster (mostly <1 ms, e.g., 0.59 ms, 0.78 ms). Allocation failures still occur but are less frequent and seem to have reduced overhead.
  2. Mark-Compact Operations:

    • Old Logs: Mark-Compact cycles appear more prolonged, with finalization phases requiring more time (e.g., 3.41 ms, 3.01 ms) and frequent triggering in response to allocation failures.
    • New Logs: Mark-Compact cycles are optimized in terms of latency (e.g., 2.08 ms, 3.01 ms). The new implementation shows slightly fewer allocation failures requiring Mark-Compact operations.
  3. Memory Usage:

    • Old Logs: The system uses more memory (e.g., up to 67.1 MB post-GC in old space).
    • New Logs: Memory usage appears better managed, with memory peaking slightly lower (e.g., 66.8 MB post-GC in old space).
  4. Overall Efficiency:

    • Old Logs: The system experiences higher cumulative latency and allocation failures, implying more frequent reliance on fallback strategies like Mark-Compact GC.
    • New Logs: Improved handling of allocation failures and faster scavenges suggest better memory management and reduced GC overhead.

Conclusions

  • Improvement in Scavenge Cycles: The new system shows significant improvement in Scavenge GC cycles, leading to faster recovery of young-generation memory.
  • Reduced Allocation Failures: Allocation failures still occur but are better handled, resulting in smoother transitions and fewer interruptions.
  • Optimized Mark-Compact GC: The Mark-Compact process, while still necessary, is more streamlined and efficient in the new system.

@RobinTail
Copy link
Owner Author

Summary of Improvements

Memory Usage Reduction: ~17.14%
GC Time Reduction: ~19.16%

@RobinTail RobinTail marked this pull request as ready for review November 15, 2024 10:56
Copy link
Owner Author

@RobinTail RobinTail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@RobinTail RobinTail merged commit bc8e9eb into make-v21 Nov 15, 2024
11 checks passed
@RobinTail RobinTail deleted the traverse-no-recursion branch November 15, 2024 12:17
RobinTail added a commit that referenced this pull request Nov 15, 2024
RobinTail added a commit that referenced this pull request Nov 20, 2024
![Kesaria Abramidze,
37](https://github.com/user-attachments/assets/31eb16f8-1e7f-40d8-83cd-3b4602eb5922)

Kesaria Abramidze, 37 years young, one of Georgia's most well-known
transgender women, was stabbed to death in her flat in the capital,
Tbilisi, just a day after the country's parliament passed a major
anti-LGBT bill.

https://www.bbc.com/news/articles/cy0lnpn019xo

Transgender women suffer too frequently from transphobic violence and
cruelty, being the less protected social group. I'd like to raise an
awareness of this problem. Humans should be creators — not killers. But
most importantly, I want every transgender girl to have an opportunity
to create applications quickly and, in general, learn to write code
easily in order to receive job offers and leave dangerously transphobic
territories for more favorable and civilized ones, and live happily
there. Protect transgender women.

-----------------------------

This version aims to improve the security and the overall development
experience. In particular, the requirements for the Express version are
increased, and you can now run the secure server exclusively.
Alternative plural properties are replaced with universal singular ones
supporting arrays. Specifying the method when creating an endpoint is
now optional, which also makes it easier to assign endpoints on a same
route for different methods. Memory consumption reduced for Routing
traverse. Previously deprecated methods and properties have been
removed, several public interfaces changed in order to enable features
coming up later.

- #2083 
- #2087 
- #2086 
- #2100 
- #2122 
- #2139 
- #2148 
- #2128 
- #2162 
- #2167 
- #2168 
- #2172 
- #2176 
- #2175 
- #2185 
- #2187 
- #2192 
- #2193 
- #2194
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring The better way to achieve the same result
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant