[PATCH] Improve talloc performance
obnox at samba.org
Thu Jun 30 21:52:22 UTC 2016
On 2016-06-30 at 10:48 -0700, Jeremy Allison wrote:
> On Wed, Jun 29, 2016 at 04:55:23PM -0700, Jeremy Allison wrote:
> > On Fri, Jun 17, 2016 at 05:12:12PM -0700, Jeremy Allison wrote:
> > > On Fri, Jun 17, 2016 at 09:50:27PM +1200, Andrew Bartlett wrote:
> > > > Attached is a patch to improve talloc performance, by calling
> > > > talloc_chunk_from_ptr() less often. I'm impressed by how much can be
> > > > gained!
> > > >
> > > > Please carefully review!
> > >
> > > I'm going through this right now, but one thing
> > > that has to be done is to split this into a set of smaller
> > > changesets as the chunks are too large to carefully parse
> > > mentally (at least for me). Too much change per patchset.
> > OK, split this into an 11-patch set, with each change
> > being somewhat clearer and smaller (IMHO of course :-).
> > Your original patchset had one error (missing setting
> > the .name in the replacement for talloc_vasprintf(),
> > and a couple of places where you added code, then removed
> > it again.
> > I'm still running tests on it at the moment, but at
> > least it compiles :-).
> > I'll post for your review once I've gotten it past
> > testing !
> OK, here it is. Passes testing and valgrind. Keeps
> the performance boost.
> The first patch renames internal functions that
> take a 'struct talloc_chunk *' from talloc_XXX()
> to tc_XXX(). Subsequent patches keep that naming
> scheme. Doing this I find it much easier to tell
> the functions that take a talloc_chunk apart from
> the ones that take the void * talloc pointer. Has
> no external effect on the ABI of course.
> The rest of them are your patch split into easily
> digestible changes.
> Next time, try and do this yourself to save me a
> lot of work (although I'd have to understand all
> the changes anyway of course :-). Small micro-patches
> the do only one thing really make this work easier.
> Please review and push if happy !
From a superficial glance, the patches look clean,
straight and reasonably-sized. But one cosmetic comment:
you seem to have dropped the actual patch authorship.
At least the singoff and review tags seem to imply that. :-)
Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 198 bytes
Desc: not available
More information about the samba-technical