Ok, I've gone ahead and implemented what I think should be a channel-based memory pooling, which you can find here:<div><br></div><div><a href="https://github.com/alanxz/rabbitmq-c/tree/channel_owns_memory_pool">https://github.com/alanxz/rabbitmq-c/tree/channel_owns_memory_pool</a></div>
<div><br></div><div>Its not quite API/ABI backwards compatible, the change that breaks things being adding an extra field to the amqp_pool_t struct:</div><div><a href="https://github.com/alanxz/rabbitmq-c/commit/2bc1dc9e22152506cf07f21d10e6a79e7ed2ccc3#diff-2">https://github.com/alanxz/rabbitmq-c/commit/2bc1dc9e22152506cf07f21d10e6a79e7ed2ccc3#diff-2</a>
</div><div><br></div><div>This could be fixed easy enough, the trade-off being some complexity internally.</div><div><br></div><div>The change isn't a small patch, and should probably have a few more pairs of eyes look at it, despite having a few unit tests written for the machinery that moves the internal memory pools around.</div>
<div><br></div><div>For an explanation of what my implementation tries to do:</div><div><a href="https://github.com/alanxz/rabbitmq-c/commit/2bc1dc9e22152506cf07f21d10e6a79e7ed2ccc3#L5R119">https://github.com/alanxz/rabbitmq-c/commit/2bc1dc9e22152506cf07f21d10e6a79e7ed2ccc3#L5R119</a>
</div><div><br></div><div>I have no idea how this will scale, especially to things like embedded devices. Hopefully I have not created a monster.</div><div><br></div><div>If the overarching idea looks sane to people, I will create a pull request and we can see about merging this in.</div>
<div><br></div><div>-Alan</div><div><br><br><div class="gmail_quote">On Fri, Jan 13, 2012 at 5:15 PM, Alexis Richardson <span dir="ltr"><<a href="mailto:alexis@rabbitmq.com">alexis@rabbitmq.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Alan,<br>
<br>
I'm afraid everyone has been distracted.<br>
<br>
Could anyone on the list opine?<br>
<span class="HOEnZb"><font color="#888888"><br>
alexis<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
On Fri, Jan 13, 2012 at 9:19 PM, Alan Antonuk <<a href="mailto:alan.antonuk@gmail.com">alan.antonuk@gmail.com</a>> wrote:<br>
> I haven't really had a huge amount of time to work on it since I sent the<br>
> first message message, with the holidays and starting up a new project at<br>
> work.<br>
><br>
> Also I was hoping to get some input from the rabbitmq guys so that any<br>
> changes I make would have a better chance of being accepted.<br>
><br>
> Taking a quick look at what I wrote a month ago, today my plan would be to<br>
> implement having a amqp_pool_t associated with each amqp_channel_t and see<br>
> how far I could get with that.<br>
><br>
> -Alan<br>
><br>
> On Thu, Jan 12, 2012 at 11:39 AM, Alexis Richardson <<a href="mailto:alexis@rabbitmq.com">alexis@rabbitmq.com</a>><br>
> wrote:<br>
>><br>
>> Alan<br>
>><br>
>> It appears that you stunned everyone into silence ;-)<br>
>><br>
>> How are you getting on with this?<br>
>><br>
>> alexis<br>
>><br>
>><br>
>> On Wed, Dec 14, 2011 at 8:15 PM, Alan Antonuk <<a href="mailto:alan.antonuk@gmail.com">alan.antonuk@gmail.com</a>><br>
>> wrote:<br>
>> > I've been working on a c++ wrapper around rabbitmq-c that presents a<br>
>> > "humane" API to the programmer heavily inspired by the Puka project.<br>
>> > (see: <a href="https://github.com/alanxz/SimpleAmqpClient" target="_blank">https://github.com/alanxz/SimpleAmqpClient</a>).<br>
>> ><br>
>> > In developing this library I've run across several limitations of the<br>
>> > rabbitmq-c when working with multiple channels, the biggest issue being:<br>
>> ><br>
>> > - There is no way to wait for a list of methods on a channel.<br>
>> > There is amqp_simple_wait_method() - however this suffers from some<br>
>> > serious<br>
>> > drawbacks:<br>
>> > + you can only specify one method to listen for<br>
>> > + it calls abort() if a different method, or a method on a different<br>
>> > channel<br>
>> > is received<br>
>> > A use case for this might be: doing a basic.publish, and you want to<br>
>> > want to<br>
>> > wait for a basic.ack or a basic.return on a channel with confirms<br>
>> > enabled<br>
>> ><br>
>> > The way I got around this in SimpleAmqpClient was to only use<br>
>> > amqp_simple_wait_frame() and maintain queues of amqp_frame_t for each<br>
>> > channel that I have open.<br>
>> ><br>
>> > However, this comes with one serious drawback: memory management. Each<br>
>> > decoded frame is allocated in a connection-wide amqp_pool_t. Because of<br>
>> > this - it is impossible to recycle the pool and release memory unless<br>
>> > you<br>
>> > have dealt with all of your pending amqp_frame_t's. This becomes a<br>
>> > problem<br>
>> > in pathological cases where you have two consumers sending<br>
>> > simultaneously,<br>
>> > you can get in the situation that even though the client program<br>
>> > eventually<br>
>> > deals with every consumed message, memory never gets released, because<br>
>> > there<br>
>> > is always at least one frame queued up.<br>
>> ><br>
>> > The above demonstrates the second biggest issue with the rabbitmq-c API:<br>
>> > memory management when dealing with multiple channels. There is no way<br>
>> > to<br>
>> > separate out memory allocation on, for example, a per-channel basis<br>
>> > (with<br>
>> > the library client keeping track of the memory pools used for example).<br>
>> ><br>
>> > Before I go on I'd like to mention one feature that I find useful with<br>
>> > the<br>
>> > current API: it is possible to use something like select() before<br>
>> > calling<br>
>> > amqp_simple_wait_frame() to setup a timeout while waiting for a<br>
>> > consumer,<br>
>> > which is useful when developing single-threaded RPC apps.<br>
>> ><br>
>> ><br>
>> > So now the interesting part: how could the API be improved?<br>
>> ><br>
>> > Some thoughts I've had dealing with the memory management:<br>
>> > 1. Create a amqp_simple_wait_frame() variant that as one of the<br>
>> > parameters<br>
>> > provides a callback to allocate memory, either something like a malloc()<br>
>> > call, which the client would then be responsible for freeing, or perhaps<br>
>> > something like get_amqp_pool(amqp_frame_t) which allows the client to<br>
>> > return<br>
>> > a memory pool which rabbitmq-c would use to allocate memory for that<br>
>> > frame.<br>
>> > The amqp_frame_t would have to have some minimal amount of information<br>
>> > filled in - such as frame type and channel to be useful.<br>
>> ><br>
>> > 2. amqp_channel_t becomes a struct containing both the the channel_id<br>
>> > and a<br>
>> > amqp_pool_t. The amqp_pool would be used by the library to allocate<br>
>> > frames<br>
>> > received on that channel. The client would then be responsible for<br>
>> > calling<br>
>> > amqp_pool_recycle at an appropriate point.<br>
>> ><br>
>> > Some thoughts on improving the API to deal with multiple channels:<br>
>> > 1. Add the following to the API:<br>
>> > amqp_simple_wait_frame_on_channel - wait for a frame on a specified<br>
>> > channel<br>
>> > amqp_simple_wait_methods - wait for multiple methods on a specified<br>
>> > channel.<br>
>> > Don't abort() if the wrong method, or channel is received, instead queue<br>
>> > up<br>
>> > the frame as is done in amqp_simple_rpc<br>
>> > amqp_frames_enqueued_for_channel to add feature parity with<br>
>> > amqp_frames_enqueued<br>
>> > I started to code this up at one point but abandoned it as it didn't<br>
>> > support<br>
>> > the interesting property of being able to use select() to specify a<br>
>> > timeout<br>
>> > when calling amqp_simple_wait_frame_on_channel. At least not without<br>
>> > adding<br>
>> > timeout to the rabbitmq-c api, which didn't fit with how the current<br>
>> > rabbitmq-c api. Here's the implementation I came up with before I<br>
>> > abandoned<br>
>> > it. I can't guarantee is completely free of<br>
>> > bugs. <a href="https://github.com/alanxz/rabbitmq-c/commits/simple_wait_frame/" target="_blank">https://github.com/alanxz/rabbitmq-c/commits/simple_wait_frame/</a><br>
>> ><br>
>> > 2. Continue to duck the issue and allow clients to write their own code<br>
>> > to<br>
>> > deal with multiple channels (fixing the memory management issue using<br>
>> > one of<br>
>> > the above techniques)<br>
>> ><br>
>> > 3. Something I haven't thought of yet.<br>
>> ><br>
>> > So anyone have any thoughts on all of this?<br>
>> ><br>
>> > -Alan<br>
>> ><br>
>> > _______________________________________________<br>
>> > rabbitmq-discuss mailing list<br>
>> > <a href="mailto:rabbitmq-discuss@lists.rabbitmq.com">rabbitmq-discuss@lists.rabbitmq.com</a><br>
>> > <a href="https://lists.rabbitmq.com/cgi-bin/mailman/listinfo/rabbitmq-discuss" target="_blank">https://lists.rabbitmq.com/cgi-bin/mailman/listinfo/rabbitmq-discuss</a><br>
>> ><br>
><br>
><br>
</div></div></blockquote></div><br></div>