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

Making socket api access transparent #1880

Closed
uttampawar opened this issue Jun 14, 2018 · 14 comments
Closed

Making socket api access transparent #1880

uttampawar opened this issue Jun 14, 2018 · 14 comments

Comments

@uttampawar
Copy link

I've been working to improve Node.js performance by various ways. One of the way is to use userspace TCP/IP stack with explicit network driver like "dpdk (http://dpdk.org)", instead of kernel stack. I've seen gain in the range of 2x for a typical Node.js application (with DB server) to 4x for small application (micro-services but no DB server) with small flow size (data transfer). But in order to do that, I'd to make socket api transparent so that it can be intercepted appropriately. This has been done by patching linux-syscall.c file for "*nix" platform.
I would like to submit the PR so that it can be pulled/integrated into Node.js project. I also wanted to have a discussion of what the community thinks of this contribution.

@bnoordhuis
Copy link
Member

Hard to say because it's not clear to me what "make socket api transparent" means. Any ABI- or API-breaking changes are certainly out of the question for v1.x but up for discussion with master.

@uttampawar
Copy link
Author

uttampawar commented Jun 14, 2018

@bnoordhuis What I mean is that current socket API (accept4, eventfd, epoll_create, etc.) is wrapped under 'syscall()' interface with specific number which makes it harder if not impossible to override or trap a particular system call. As an example if libuv were to call 'epoll_create()' directly instead of via syscall() then it would be easier to trap it in the userspace stack. Here is a snapshot of the change as an example,

int uv__epoll_create(int size) {
#if defined(__NR_epoll_create)
#if defined(MTCP_DPDK)
return epoll_create(size);
#else
return syscall(__NR_epoll_create, size);
#endif
#else
return errno = ENOSYS, -1;
#endif
}

Change is under #MTCP_DPDK

There are many userspace stacks available (such as mTCP, seastar, F-stack, LKL, etc.) Currently I've integrated libuv with userspace stack called 'mTCP' which is the closest BSD compatible stack among them with best performance so far.

@bnoordhuis
Copy link
Member

See #1372, we'll probably get rid of the wrappers in due time. They exist to deal with old kernel/libc versions but will be phased out over time.

On a tangent:

[..] 'syscall()' interface with specific number which makes it harder if not impossible to override or trap a particular system call.

I did just that in StrongLoop's strong-agent tool, by interposing syscall. There's no reason it can't work.

@uttampawar
Copy link
Author

My changes won't require, when nodejs/node#13306 lands.

@richardlau
Copy link
Contributor

All currently supported versions of Node.js contain a later version of libuv than 1.12.0 in the referenced nodejs/node#13306.

@uttampawar
Copy link
Author

@richardlau If I understand correctly, this change has not yet been merged or back ported in libuv v1.x and hence in Node.js. In this case I prefer to implement syscall() interface as @bnoordhuis suggested, i.e. no changes to libuv. Could you please share your thoughts?

@bnoordhuis
Copy link
Member

In case you're curious what strong-agent did: https://github.com/strongloop/strong-agent/blob/1f5a1aae2eaff0f6833e4d587cbcdf0e0aa646b8/src/watchdog.h#L105-L204

Caveat emptor: proprietary code.

@uttampawar
Copy link
Author

@bnoordhuis Thanks.

@richardlau
Copy link
Contributor

@uttampawar I'm afraid I'm not following you. You stated that:

My changes won't require, when nodejs/node#13306 lands.

But nodejs/node#13306 is the upgrade in Node.js to libuv 1.12.0. All current releases of Node.js have later versions of libuv. If the change you require isn't in libuv v1.x then I'm confused as to your earlier statement.

@uttampawar
Copy link
Author

@richardlau I misunderstood the whole story. I thought that syscall wrapper change is already merged into libuv v1.12.0 (part of #1372) and into Node.js by PR nodejs/node#13306 . Then last night I realized my mistake. Sorry about the confusion.

@uttampawar
Copy link
Author

@bnoordhuis @richardlau Just curious, do you have time estimate when #1372 will merge into v1.x?

@bnoordhuis
Copy link
Member

No.

@stale
Copy link

stale bot commented Aug 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 10, 2019
@stale stale bot closed this as completed Aug 17, 2019
@unicomp21
Copy link

Can someone clue me in on the current "state of the art" with respect to libuv, nodejs, and userspace tcp?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants