Do not create string objects from consumerTag, exchange and routingKey, or get them from a string cache.

[zgabi]

In our real application the most allocated objects are Strings. Most of them are allocated in RabbitMQ Client:

They usually contains the same value.

Please consider using ReadOnlySpan<char> or ReadOnlySpan<byte> with a pooled array in the background similar to the data.
Alternatively get the strings from a cache table (maybe separate table for each "string type" (consumerTag, exchange...)), since most of them are the same.

By the way in the screenshot the 3rd, 4th, 5th and 6th allocations also belongs to RabbitMQ Client, but maybe some of them are already solved in the main branch. (I'm using the latest 6.x package)
The 2nd one is WFP, I have a PR for that.

[michaelklishin]

How much difference would this make to even a very basic (tutorial 1-2 style) consumer?

[zgabi]

I don't undertand your question. We have 1 consumer in this process.

[lukebakken]

Feel free to submit a pull request implementing your suggestion.

[zgabi]

Which one do you prefer?

1. Receive ReadOnlySpan<char> in the IBasicConsumer's Handle* methods (e.g HandleBasicDeliver), so it is already UTF-8 decoded characters
2. Receive ReadOnlySpan<byte> in the IBasicConsumer's Handle* methods (e.g HandleBasicDeliver), the raw bytes
3. Do not change the interface, but cache the strings in WireFormatting.ReadShortstr.. maybe this one is slower, because the cached strings should be compared with the received data. But do not require to change the interface.

[zgabi]

In the framework? dotnet/runtime#28368

[lukebakken]

No for some reason I thought string interning worked differently.

I'd like to get feedback from the "usual gang", aka @bollhals, @bording, @danielmarbach and @stebet if they have time. I'm a bit surprised nobody has reported this issue in their use of this library.

[michaelklishin]

@zgabi my point is that caching is always harder than it sounds. What would be the gains (in terms of memory footprint and allocations) to a realistic consumer (application) like yours, or even a basic tutorial-style one.

Previously all efficiency improvements in this client were driven by profiler results. Sure,
you already have some but I am curious about the difference it makes.

[zgabi]

I'd like to get feedback from the "usual gang", aka @bollhals, @bording, @danielmarbach and @stebet if they have time. I'm a bit surprised nobody has reported this issue in their use of this library.

Because they are just some small allocations. 2-3 small strings / message ("exchange" is empty string for me, which is not allocated)
And everybody is happy since a lot of other allocations were removed in the past 3-4 years. (Me, too) Version 6.x is much better and faster than 4.x was 4 years ago. And I saw that the main branch has more new related improvelemts (BasicProperties is a struct, etc...)

We are sending a lot of messages (~200/ sec), because it is a requirement for us to send everything between the server and the clients through RabbitMQ... even video streams.
This string allocation problem is also not a big issue for us... 1.8 M messages allocations / 38 minutes. GC can handle it :)

I reporteed this issue, since it is the top 1 in the memory profiler.

[zgabi]

Yes, caching is harder than simply passing spans to the user.

For exmaple:

• caculating some hash from the data
• put it to a thread static dictionary (with this the string will exist only 1 time / thread, which is quite good, and does not need locking).

So probably this is slower than just simply creating a new string.

So I'd prefer passing span (or memory) of byte to the user.... similar to the data.
It needs the modification of the IBasicConsumer, but does not have any other disadvantages. It will be even faster, since no UTF-8 decoding is needed. And no string allocation, although it is fast...

[michaelklishin]

@zgabi we can do this for 7.0. Option 2 above is more in the spirit of RabbitMQ and clients but I can see how many might vote for option 1.

[lukebakken]

I agree with @michaelklishin that this should be 7.0 only if there is an API change.