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

Consider pointer based socket for performance #1724

Open
gdamore opened this issue Dec 15, 2023 · 3 comments
Open

Consider pointer based socket for performance #1724

gdamore opened this issue Dec 15, 2023 · 3 comments
Milestone

Comments

@gdamore
Copy link
Contributor

gdamore commented Dec 15, 2023

Today we use integers and hash table (which must be locked) with reference counting etc. to guarantee no use-after-free, when referring to sockets or contexts.

For many applications, we don't need the strict requirement that attempting to use a socket after it is closed is safe, and we could allow a paradigm where the finalization of the socket is the responsibility of the application. Further, we could require that application not destroy a socket unless it is certain that no other use is present - i.e. deferring the requirement for thread-safety for this particular operation to the application.

This should be an opt-in API, with suitable warnings. But for applications that can make use of them (for example many server applications open a socket and never close it), this would potentially greatly increase performance as we would no longer have to pay the price for the hash table lookups, nor the locking overhead associated with those lookups and reference counts.

This would clearly be a trade-off of convenience & safety in favor of raw performance. There are applications willing to make that trade-off.

@gdamore
Copy link
Contributor Author

gdamore commented Dec 15, 2023

Note that use of this API would necessarily need to be incompatible with the existing API -- no intermingling of sockets of both unsafe and safe variants, and likely certain APIs which need to get references on objects would be difficult to support -- for example the statistics API would probably not be supported, or would only be supported for a given socket not for global views. We might also restrict what information is available for a pipe on a socket.

It is possible that we could give an option to get an unsafe handle from an existing safe socket, which might allow some intermingling as well.

This could work like:

unsafe_socket *nng_unsafe_socket_handle(nng_socket_t sock);

This would acquire a reference count on the socket, which would be held until the socket is released, which would have to be done by the user explicitly. Then we would have unsafe_socket versions of send and receive. We probably don't need to support unsafe versions of option APIs or others, because they are not normally on the hot code path.

@gdamore
Copy link
Contributor Author

gdamore commented Nov 4, 2024

I've had an inspiration.

What I want to do is add a new field to nng_socket, that is a pointer. If this is not null, then we can use it. Otherwise we have to do the old lookup.

We'll need a new constructor...

nng_req0_create() or similar. This will work like open, but grab a hold on the socket and store the pointer.

A public nng_socket_hold() function could be used to convert an id to a held with pointer structure.

When we're done, nng_socket_release() can release the socket. (Or possibly we can make nng_close() do it, but we need to be very clear about the semantics here -- is there any expectation for ability to use after close?

Internally, nng_sock_find() can be aware of this, and simply not do the locking or the table lookup if it finds the field is already populated.

This can also be done for contexts, where it will gain big, and possibly also listeners and dialers, but I doubt it will pay off much for listeners and dialers since those accesses (and also pipes) are probably infrequent.

@gdamore gdamore added this to the v2.0 milestone Nov 4, 2024
@gdamore
Copy link
Contributor Author

gdamore commented Nov 4, 2024

The approach given above would not be unsafe.

@gdamore gdamore changed the title Consider pointer based socket (unsafe) for performance Consider pointer based socket for performance Nov 4, 2024
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

1 participant