Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-26 Thread Arjun Sreedharan
On 26 August 2014 02:44, Junio C Hamano gits...@pobox.com wrote: Arjun Sreedharan arjun...@gmail.com writes: Find and allocate the required amount instead of allocating extra 100 bytes Signed-off-by: Arjun Sreedharan arjun...@gmail.com --- Interesting. How much memory do we typically

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-26 Thread Jeff King
On Mon, Aug 25, 2014 at 02:36:02PM -0700, Junio C Hamano wrote: I think you are right, and the patch is the right direction (assuming we want to do this; I question whether there are enough elements in the list for us to care about the size, and if there are, we are probably better off

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-26 Thread Ramsay Jones
On 26/08/14 12:03, Jeff King wrote: [snip] Yeah, reading my suggestion again, it should clearly be alloc_flex_struct or something. Here's a fully-converted sample spot, where I think there's a slight benefit: diff --git a/remote.c b/remote.c index 3d6c86a..ba32d40 100644 ---

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-26 Thread Jeff King
On Tue, Aug 26, 2014 at 12:57:21PM +0100, Ramsay Jones wrote: + ret = xcalloc(1, base + extra); + va_start(ap, fmt); + vsnprintf(ret + offset, extra, fmt, ap); What is the relationship between 'base' and 'offset'? Let me assume that base is always, depending on your compiler,

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-26 Thread Ramsay Jones
On 26/08/14 13:14, Jeff King wrote: On Tue, Aug 26, 2014 at 12:57:21PM +0100, Ramsay Jones wrote: + ret = xcalloc(1, base + extra); + va_start(ap, fmt); + vsnprintf(ret + offset, extra, fmt, ap); What is the relationship between 'base' and 'offset'? Let me assume that base is

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-26 Thread Jeff King
On Tue, Aug 26, 2014 at 01:37:44PM +0100, Ramsay Jones wrote: On my 64-bit system using gcc, sizeof() returns 16; it has to pad the whole thing to 64-bit alignment in case I put two of them in an array. But offsetof(name) is 12, since the array of char does not need the same alignment; it

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-26 Thread Ramsay Jones
On 26/08/14 13:43, Jeff King wrote: On Tue, Aug 26, 2014 at 01:37:44PM +0100, Ramsay Jones wrote: On my 64-bit system using gcc, sizeof() returns 16; it has to pad the whole thing to 64-bit alignment in case I put two of them in an array. But offsetof(name) is 12, since the array of char

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-25 Thread Jeff King
On Sun, Aug 24, 2014 at 04:39:37PM -0700, Junio C Hamano wrote: On Sun, Aug 24, 2014 at 8:10 AM, Stefan Beller stefanbel...@gmail.com wrote: for (p = list, i = 0; i cnt; i++) { - struct name_decoration *r = xmalloc(sizeof(*r) + 100); + char name[100];

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-25 Thread Jeff King
On Sun, Aug 24, 2014 at 07:47:24PM +0530, Arjun Sreedharan wrote: diff --git a/bisect.c b/bisect.c index d6e851d..c96aab0 100644 --- a/bisect.c +++ b/bisect.c @@ -215,10 +215,13 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n } qsort(array,

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-25 Thread Christian Couder
On Mon, Aug 25, 2014 at 3:35 PM, Jeff King p...@peff.net wrote: On Sun, Aug 24, 2014 at 07:47:24PM +0530, Arjun Sreedharan wrote: diff --git a/bisect.c b/bisect.c index d6e851d..c96aab0 100644 --- a/bisect.c +++ b/bisect.c @@ -215,10 +215,13 @@ static struct commit_list

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-25 Thread Jeff King
On Mon, Aug 25, 2014 at 04:06:52PM +0200, Christian Couder wrote: This allocation should be name_len + 1 for the NUL-terminator, no? I wondered about that too, but as struct name_decoration is defined like this: struct name_decoration { struct name_decoration *next; int

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-25 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Mon, Aug 25, 2014 at 04:06:52PM +0200, Christian Couder wrote: This allocation should be name_len + 1 for the NUL-terminator, no? I wondered about that too, but as struct name_decoration is defined like this: struct name_decoration { struct

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-25 Thread Jeff King
On Mon, Aug 25, 2014 at 11:26:39AM -0700, Junio C Hamano wrote: Good digging, and I agree that it should use the FLEX_ARRAY for consistency. I can produce a patch, but I did not want to steal Arjun's thunder. Arjun, did my proposal make sense? Do you want to try implementing that? -Peff --

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-25 Thread Arjun Sreedharan
On 26 August 2014 01:05, Jeff King p...@peff.net wrote: On Mon, Aug 25, 2014 at 11:26:39AM -0700, Junio C Hamano wrote: Good digging, and I agree that it should use the FLEX_ARRAY for consistency. I can produce a patch, but I did not want to steal Arjun's thunder. Please feel free to do so.

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-25 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Mon, Aug 25, 2014 at 11:26:39AM -0700, Junio C Hamano wrote: Good digging, and I agree that it should use the FLEX_ARRAY for consistency. I can produce a patch, but I did not want to steal Arjun's thunder. Hmph, would it have to overlap? I think we can

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-25 Thread Junio C Hamano
Arjun Sreedharan arjun...@gmail.com writes: Find and allocate the required amount instead of allocating extra 100 bytes Signed-off-by: Arjun Sreedharan arjun...@gmail.com --- Interesting. How much memory do we typically waste (in other words, how big did cnt grew in your repository where

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-25 Thread Junio C Hamano
Jeff King p...@peff.net writes: The string will always be dist= followed by decimal representation of a count that fits in int anyway, so I actually think use of strbuf is way overkill (and formatting it twice also is); the patch as posted should be just fine. I think you are right, and the

[PATCH] bisect: save heap memory. allocate only the required amount

2014-08-24 Thread Arjun Sreedharan
Find and allocate the required amount instead of allocating extra 100 bytes Signed-off-by: Arjun Sreedharan arjun...@gmail.com --- bisect.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index d6e851d..c96aab0 100644 --- a/bisect.c +++ b/bisect.c

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-24 Thread Stefan Beller
On 24.08.2014 16:17, Arjun Sreedharan wrote: Find and allocate the required amount instead of allocating extra 100 bytes Signed-off-by: Arjun Sreedharan arjun...@gmail.com --- bisect.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-24 Thread Ramsay Jones
On 24/08/14 15:17, Arjun Sreedharan wrote: Find and allocate the required amount instead of allocating extra 100 bytes Signed-off-by: Arjun Sreedharan arjun...@gmail.com --- bisect.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index

[PATCH] bisect: save heap memory. allocate only the required amount

2014-08-24 Thread Arjun Sreedharan
find and allocate the required amount instead of allocating extra 100 bytes Signed-off-by: Arjun Sreedharan arjun...@gmail.com --- bisect.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index d6e851d..a52631e 100644 --- a/bisect.c +++ b/bisect.c

Re: [PATCH] bisect: save heap memory. allocate only the required amount

2014-08-24 Thread Junio C Hamano
On Sun, Aug 24, 2014 at 8:10 AM, Stefan Beller stefanbel...@gmail.com wrote: for (p = list, i = 0; i cnt; i++) { - struct name_decoration *r = xmalloc(sizeof(*r) + 100); + char name[100]; Would it make sense to convert the 'name' into a git strbuf? Please have