search

Azure / azure-amqp

AMQP C# library
Other
94 stars 70 forks source link

Expose data segment without having to box #184

Closed danielmarbach closed 3 years ago

danielmarbach commented 3 years ago

With this change, it is possible to access the data segment directly without having to box the array segment struct. To make things backward compatible I had to make the base class Value property virtual.

danielmarbach commented 2 years ago

@xinchen10 I see that this change has never made it into a released version. This is a bummer because it would allow reducing quite a bit of boxing of ArraySegments in AzureServiceBus lib.

Is it possible to bring this on a next minor? // cc @JoshLove-msft @jsquire

xinchen10 commented 2 years ago

Do you know how much allocations it saves in a typical SB scenario? Though I don't have the data, I don't think it is significant compared to other allocations the library and SB SDK have. That's why we didn't take it for releases. It is a good change. If @JoshLove-msft @jsquire want to take it, we will add it for the next minor. To take advantage of it, the SDK needs to make a small change.

danielmarbach commented 2 years ago

As it stands today for all outgoing messages a Data frame is created and an ArraySegment assigned to the Value property. I always intended to bring back this change to the SDK to remove this boxing allocation for all outgoing sends.

Of course there are other allocations too that are worth addressing yet this change still merits to be brought to light, wouldn't it? After all, it is a bit sad to have optimizations lingering around in a not released state.

danielmarbach commented 2 years ago

@JoshLove-msft @jsquire thoughts on the above? :point_up: