Skip to content

Conversation

@anonrig
Copy link
Member

@anonrig anonrig commented Oct 30, 2025

We don't need to call RAND_BYTES for each call. We can batch them to reduce the overhead and improve performance.

@anonrig anonrig requested review from a team as code owners October 30, 2025 15:26
@kentonv
Copy link
Member

kentonv commented Oct 30, 2025

I think we should figure out exactly why RAND_bytes is so slow, and if it can't easily be fixed, maybe implement a more global wrapper for random reads that can benefit the whole process instead of just this one function. Perhaps we should have thread_local random buffers or something.

@anonrig
Copy link
Member Author

anonrig commented Oct 30, 2025

I think we should figure out exactly why RAND_bytes is so slow, and if it can't easily be fixed, maybe implement a more global wrapper for random reads that can benefit the whole process instead of just this one function. Perhaps we should have thread_local random buffers or something.

This is what I had in mind as well, but I'm not sure if it would be "safe" to store pre-generated random values in JS heap to be used whenever it's needed. I've did a similar optimization in Node.js almost 3 years ago but blocked due to security concerns. nodejs/node#44661

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 30, 2025

CodSpeed Performance Report

Merging #5442 will improve performances by 8.17%

Comparing yagiz/improve-rand-sys-calls (b271965) with main (c255a8c)

Summary

⚡ 1 improvement
✅ 33 untouched
⏩ 9 skipped1

Benchmarks breakdown

Benchmark BASE HEAD Change
request[GlobalScopeBenchmark] 58.7 ms 54.2 ms +8.17%

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@anonrig
Copy link
Member Author

anonrig commented Oct 30, 2025

@kentonv I've taken your recommendation and opened another pull-request: #5444

@anonrig
Copy link
Member Author

anonrig commented Oct 30, 2025

Actually I think we should merge these 2 pull-request since there are places where entrophysource does not exist, and in those areas batched requests are extremely valuable.

@anonrig
Copy link
Member Author

anonrig commented Oct 30, 2025

Closing in favor of #5444

@anonrig anonrig closed this Oct 30, 2025
@anonrig anonrig deleted the yagiz/improve-rand-sys-calls branch October 30, 2025 16:42
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.

2 participants