Bug in talloc_asprintf_append()
jra at samba.org
Fri Sep 14 22:24:40 GMT 2007
On Sat, Sep 15, 2007 at 08:22:11AM +1000, tridge at samba.org wrote:
> This function came from Samba4, and we depend on the no strlen
> behaviour in several places there.
> The reason it matters is that talloc_asprintf_append() was originally
> added for use in places where we build up strings in a loop. For
> example, its used to construct ldap entries and the schema. These
> strings can be very large. With a strlen() call the time was O(n^2) in
> the length of the string, whereas without the strlen() the time is
> O(n) in the length of the string. That made a huge difference as n is
> sometimes many thousands. At one stage this was the top of the list in
> many profiles.
> I don't mind it being renamed, as long as we have the original
> functionality available somewhere.
> This will also affect the performance of Samba3 to some extent. See
> for example the msdfs.c code (where its used in a loop), the
> pdb_ldap.c code (another loop) net_conf.c (yet, another loop).
> In fact, it looks like using talloc_asprintf_append() in a loop to
> build a string is more common in Samba3 than using it as a one-off. So
> by adding that strlen() you are potentially adding quite a high cost
> to those loops. The average loop count is probably quite low in most
> of the places in Samba3, but its still an awful lots of CPU cycles
> So at the very least all those functions that use it in a loop should
> be changed to use the varient that doesn't do the strlen(). This is a
> case where talloc wins us a lot of cycles by knowing the length of a
> piece of allocated memory, I don't think we should throw that away.
I've already made this change thanks :-). Volker beat me up about it
talloc_asprintf_append() uses strlen, talloc_asprintf_append_buffer
Do you want me to make the changes in Samba4 also ?
More information about the samba-technical