talloc: Other minor issues/queries

[Jeremy Allison]

On Wed, Oct 21, 2020 at 06:51:04PM -0500, Arran Cudbard-Bell wrote:
> 
> Would references would be on the chopping block for a new major
> release also? I have to say that's one of the features we also use
> but one we could easily replicate internally and wouldn't really mourn.

I would *love* to get rid of references. I once had a long
conversation with Josh Bloch:

https://en.wikipedia.org/wiki/Joshua_Bloch

who used to be an office-mate of mine (one of
the benefits of working for Google :-). As he's
an expert in API design I ran the talloc API
past him.

He said it was mostly OK but *hated* talloc_reference() :-).

Unfortunately we'd need to get rid of it from Samba
first. Something I've been working (slowly:-) towards
for years...

> Now we're discussing major revs, there are a couple of other issues 
> we've run into that it'd be good to get comment on.
> 
> First, is that the lack of parent pointer for the majority of chunks makes
> any call that needs to access the parent ctx, very slow, when there 
> are a large number of chunks at the same level.
> 
> I'm assuming it's a trade off between cheap reallocs and cheap
> talloc_steal/talloc_parent_*.  Is allocing a bunch of chunks off of a 
> context that gets realloced a common pattern for Samba? Am I 
> missing something else that makes recording the parent in each chunk
> painful?

It's probably a size optimization to avoid bloating out
the talloc chunk header more than we need.

We tend not to reparent or realloc much. Just allocate
off a context and children and then free it.

> I mention this because we had to strip out almost all calls to
> talloc_steal and talloc_parent*, because they featured so heavily
> in code profiling.  If it really is just a tradeoff between realloc and
> talloc_parent* it might be good to revisit how parents are 
> recorded.
>
> Second, is expandable pools.
> 
> In FreeRADIUS the instance data for modules is only protected 
> when a user sets a special environmental variable to specify the pool
> size for all instance data memory. The user has to guess at how 
> big this pool needs to be (which is pretty terrible) but it's the only way 
> to handle this with the current version of talloc.
> 
> When we're building up the dictionaries we have a similar problem
> in that we need to guess at how big the pool needs to be because
> we can't expand it.  This leaves the pool massively oversized in the
> majority of cases.

Pools are really designed to be used for *known* data sizes.

Look at how we use them for filename in cp_smb_filename()
via talloc_pooled_object().

Remember, talloc is our internal library that others found
useful, so we design it around our needs first.

Selfish, but that's just the way it is :-).

> Having talloc automatically expand pools would be amazing 
> (imagining a linked list of fixed size malloced chunks).  I know there's
> additional complexity here, but it does seem to be limited to very
> specific areas of the code.
> Would this functionality ever be considered?

Be very clear about what you want this to look like.

Are pools the right thing ?