On Tue, Jun 7, 2011 at 7:08 PM, Joachim Bauch <j...@struktur.de> wrote: > Am Montag, den 06.06.2011, 20:29 -0400 schrieb Nick Mathewson: >> Hm. If there's nothing hideously complex in the source buffer (that >> is, nothing added with evbuffer_add_file), you could try an >> appropriate combination of evbuffer_peek and evbuffer_add_reference. >> Other than that, I'm not thinking of much. >> >> I'd love to get a better API for this in 2.1, if anybody wants to write one. > > Attached is a patch against the current git head that adds a new method > "evbuffer_add_buffer_reference" providing a non-destructive add from one > evbuffer to another while avoiding unnecessary copies. For now it only > supports buffers containing only memory chains (i.e. no file segments > and sendfile chains). Also included is a first testcase, the default > regression tests still pass.
Neat! So if I understand here, the semantics are that once a chain has been added by reference to another evbuffer, the chain is now supposed to be immutable. Could we simplify stuff by also declaring that if you have an evbuffer which is added by reference to another evbuffer, the entire original evbuffer is immutable? It seems to me that would make the rules for what you can use these for much easier to understand, and it would prevent people from doing crazy stuff. Is there a reason that EVBUFFER_MEM_PINNED_M needs to be distinct from EVBUFFER_IMMUTABLE? I think there's a race condition around evbuffer_chain_decref. The reference count needs to be protected by the lock of the evbuffer that contains the referenced-chain, but right now it looks like it's only protected by the lock of the evbuffer that contains the referencing chain. I think we can move the "parent" field into a type-specific EVBUFFER_CHAIN_EXTRA() annex, in the way that evbuffer_chain_reference and evbuffer_chain_file_segment and evbuffer_file_segment currently do. It would be fine to do this one in another patch, but evbuffer_file_segment should actually be copyable with the appropriate bookkeeping. Also, I had considered another possible implementation. Instead of adding a reference to each chain in the referenced buffer, we could instead add a reference to the referenced buffer itself, and use an evbuffer_ptr to track how far into it we were. But on consideration, I think your approach is better: my idea would make too many functions harder to implement, without much real gain. > As this is my first attempt to change something in the buffer internals, > could you please check for any obvious errors or wrong coding style? > I hope sending this to the list is okay, or should I post it somewhere > else? The list is a great place to discuss patches, but IMO it has the problem that stuff can get forgotten. Sourceforge and github are better at preventing stuff from getting forgotten, but fewer people read them, alas. Let's keep talking here for now, but if one of us drops the ball on replying, let's remember to add a ticket/pull request/whatever for this. Also fwiw, the best format for sending patches is the one generated by "git format-patch"; it makes them much easier to merge. Pointing at a branch on a public git repository is also pretty good. many thanks, -- Nick *********************************************************************** To unsubscribe, send an e-mail to majord...@freehaven.net with unsubscribe libevent-users in the body.
