Skip to content

Conversation

@theanarkh
Copy link
Contributor

Add an option for using recvmmsg.

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 19, 2025
@theanarkh theanarkh changed the title dgram: add option or recvmmsg dgram: add option for using recvmmsg Jul 19, 2025
@theanarkh theanarkh force-pushed the support_using_recvmmsg branch from ecc5df0 to fd1ca64 Compare July 19, 2025 21:09
@codecov
Copy link

codecov bot commented Jul 19, 2025

Codecov Report

❌ Patch coverage is 83.07692% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.55%. Comparing base (049664b) to head (aac2964).
⚠️ Report is 608 commits behind head on main.

Files with missing lines Patch % Lines
src/udp_wrap.cc 80.48% 4 Missing and 4 partials ⚠️
src/udp_wrap.h 76.92% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59126      +/-   ##
==========================================
- Coverage   90.06%   88.55%   -1.51%     
==========================================
  Files         645      704      +59     
  Lines      189130   208142   +19012     
  Branches    37094    40027    +2933     
==========================================
+ Hits       170339   184323   +13984     
- Misses      11511    15828    +4317     
- Partials     7280     7991     +711     
Files with missing lines Coverage Δ
lib/dgram.js 97.46% <100.00%> (+0.01%) ⬆️
lib/internal/dgram.js 100.00% <100.00%> (ø)
src/udp_wrap.h 85.71% <76.92%> (-14.29%) ⬇️
src/udp_wrap.cc 78.30% <80.48%> (+<0.01%) ⬆️

... and 329 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@theanarkh theanarkh force-pushed the support_using_recvmmsg branch from fd1ca64 to d09c31a Compare July 20, 2025 06:31
@theanarkh theanarkh added the dgram Issues and PRs related to the dgram subsystem / UDP. label Jul 25, 2025
src/udp_wrap.cc Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Can these fieds be initialized as part of the constructor member initializer list or inline in the member declaration?

Copy link
Member

Choose a reason for hiding this comment

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

What if this multiplication overflows?

src/udp_wrap.cc Outdated
Copy link
Member

Choose a reason for hiding this comment

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

What if malloc() returns 0?

Copy link
Member

Choose a reason for hiding this comment

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

Can we CHECK() in this case that mmsg_buf_ is empty?

src/udp_wrap.h Outdated
Copy link
Member

Choose a reason for hiding this comment

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

^ This should track mmsg_buf_

@theanarkh theanarkh force-pushed the support_using_recvmmsg branch from d09c31a to 89c252e Compare October 9, 2025 17:58
@theanarkh
Copy link
Contributor Author

@addaleax Thanks for your review ! I have updated the code.

@theanarkh theanarkh force-pushed the support_using_recvmmsg branch from 89c252e to aac2964 Compare October 9, 2025 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. dgram Issues and PRs related to the dgram subsystem / UDP. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants