[PATCH] Avoid some memory waste while allocating tcbtab.
Nahim El Atmani
nahim+dev at naam.me
Tue Mar 1 16:44:57 UTC 2016
Hello,
On Tue, 01 Mar 2016 02:27:09 +0300, Dmitry V. Levin wrote:
> [...]
> > + if (!tcbtabsize) {
> > + create_tcbtab();
> > + } else {
> > + i = tcbtabsize;
> > + newtcbs = xcalloc(tcbtabsize, sizeof(newtcbs[0]));
> > + newtab = xreallocarray(tcbtab, tcbtabsize * 2,
> > + sizeof(tcbtab[0]));
> > + tcbtabsize *= 2;
> > + tcbtab = newtab;
> > + while (i < tcbtabsize)
> > + tcbtab[i++] = newtcbs++;
> > + }
> > }
>
> This is better. Do you think create_tcbtab that's called just once
> from expand_tcbtab worth to be a separate function?
I mainly did that for visibility and testing purposes at first (since it's
easier to test small atomic chunks of code than a big fully featured function).
I agree, however, that there is no advantages of spliting the code in this
case since the code is covered in both situations. Fixed in the next version.
>
> What if these two branches of code were merged, e.g.
>
> unsigned int new_tcbtabsize, alloc_tcbtabsize;
> if (tcbtabsize) {
> alloc_tcbtabsize = tcbtabsize;
> new_tcbtabsize = tcbtabsize << 1;
> } else {
> alloc_tcbtabsize = new_tcbtabsize = 1;
> }
> etc.
>
> Wouldn't the result code be more compact and somewhat more readable?
I would say that it's more compact for sure, but I still have some doubts
concerning the readability since it introduces some less obvious behaviors. As
an illustration let's take the complete example:
> unsigned int i, new_tcbtabsize, alloc_tcbtabsize;
> struct tcb *newtcbs;
> struct tcb **newtab;
>
> if (tcbtabsize) {
> alloc_tcbtabsize = tcbtabsize;
> new_tcbtabsize = tcbtabsize * 2;
> } else {
> new_tcbtabsize = alloc_tcbtabsize = 1;
> }
>
> i = tcbtabsize;
> newtcbs = xcalloc(alloc_tcbtabsize, sizeof(newtcbs[0]));
> newtab = xreallocarray(tcbtab, new_tcbtabsize, sizeof(tcbtab[0]));
Here xreallocarray() is used as a xmalloc() since tcbtab lives in the bss and
was not defined yet. Nothing wrong with that except that one have to get the
whole picture before understanding this code.
Nevertheless, I think this version is the right way to do it. I tried to get
the most readable version as possible, but it's far from being the most compact
one. I'm sending a new version.
Regards,
--
Nahim El Atmani <nahim+dev at naam.me>
http://naam.me/
More information about the Strace-devel
mailing list