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

Stop allocating strings for every ParameterStatus message #2236

Open
roji opened this Issue Nov 20, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@roji
Member

roji commented Nov 20, 2018

When a connection parameter changes, PostgreSQL sends a ParameterStatus to notify the client. It turns out that when we do DISCARD ALL when returning connections to the pool, PostgreSQL sends is_superuser and session_authorization. The current implementation when receiving ParameterStatus is to allocate a string for the key and value, so we have lots of needless allocations.

The parameter names can probably be interned (very short list), and for the value we can simply check if there was a change compared to the current value.

@roji roji added the performance label Nov 20, 2018

@roji roji added this to the 4.1 milestone Nov 20, 2018

@austindrenski

This comment has been minimized.

Member

austindrenski commented Nov 20, 2018

I'm unfamiliar with this area of the code base... but does ReadOnlySpan<char> have any role to play here?

For example, refactoring NpgsqlReadBuffer.ReadNullTerminatedString(...) to populate and return ReadOnlySpan<char> instead of string?

@roji

This comment has been minimized.

Member

roji commented Nov 21, 2018

It could help. However... The point of parameters is also to have a key-value dictionary which can be consulted at any time to get the state of any parameter - both by Npgsql internal, and by external users: we expose a read-only PostgresParameters dictionary on NpgsqlConnection. Since Span is a ref struct which cannot exist on the heap, it's only a temporary parsing mechanism... At the end of the day we still want a Dictionary<string,string> which we can look at easily.

But I think this is pretty easy to solve regardless of Span... As long as we cache the byte[] representation of a parameter somewhere, we can compare incoming values in ParameterStatus against the cache, and if there's no change, we do nothing at all. If there is one, we can pay the allocation cost and have a string as usual - I think that should be very acceptable. The main goal IMHO is to avoid allocations when the parameter value does not change.

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