| Message ID | 20200811230543.2169774-4-keithp@keithp.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Aug 11 16:05, Keith Packard via Newlib wrote: > This reduces memory usage when reallocating objects much smaller. > > Signed-off-by: Keith Packard <keithp@keithp.com> > --- > newlib/libc/stdlib/nano-mallocr.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/newlib/libc/stdlib/nano-mallocr.c b/newlib/libc/stdlib/nano-mallocr.c > index cef23977e..83234c618 100644 > --- a/newlib/libc/stdlib/nano-mallocr.c > +++ b/newlib/libc/stdlib/nano-mallocr.c > @@ -476,10 +476,8 @@ void * nano_realloc(RARG void * ptr, malloc_size_t size) > return NULL; > } > > - /* TODO: There is chance to shrink the chunk if newly requested > - * size is much small */ > old_size = nano_malloc_usable_size(RCALL ptr); > - if (old_size >= size) > + if (size <= old_size && (old_size >> 1) < size) > return ptr; > > mem = nano_malloc(RCALL size); > -- > 2.28.0 Ok, so this patch introduces the need for the conditional I complained about in the previous patch. IMO, the conditional should be moved into this patch to introduce it together with the change it requires. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer Red Hat
Corinna Vinschen via Newlib <newlib@sourceware.org> writes: > Ok, so this patch introduces the need for the conditional I complained > about in the previous patch. IMO, the conditional should be moved > into this patch to introduce it together with the change it requires. I'm easy; just depends on what you want the history to look like. -- -keith
On Aug 12 08:01, Keith Packard via Newlib wrote: > Corinna Vinschen via Newlib <newlib@sourceware.org> writes: > > > Ok, so this patch introduces the need for the conditional I complained > > about in the previous patch. IMO, the conditional should be moved > > into this patch to introduce it together with the change it requires. > > I'm easy; just depends on what you want the history to look like. Just as I wrote above. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer Red Hat
Corinna Vinschen <vinschen@redhat.com> writes:
> Just as I wrote above.
Done. In reviewing this code further, I've done a bunch more
restructuring to make the code 64-bit clean, removing the 'offset'
mechanism and letting the compiler compute alignment requirements for
us. The diff for that is pretty large though; would there be an interest
in looking at it? I ask because preparing that patch for newlib would
take several hours as it's currently only in picolibc.
https://github.com/keith-packard/picolibc/commit/fd2d18bb5ab442f16789c243648d07b4ec8e2b29
This also uncovers problems when building with newer GCC versions which
"helpfully optimize" common malloc/free operations that cause incorrect
code to be generated in some configurations of the library.
--
-keith
On Aug 13 17:48, Keith Packard via Newlib wrote: > Corinna Vinschen <vinschen@redhat.com> writes: > > > Just as I wrote above. > > Done. In reviewing this code further, I've done a bunch more > restructuring to make the code 64-bit clean, removing the 'offset' > mechanism and letting the compiler compute alignment requirements for > us. The diff for that is pretty large though; would there be an interest > in looking at it? I ask because preparing that patch for newlib would > take several hours as it's currently only in picolibc. > > https://github.com/keith-packard/picolibc/commit/fd2d18bb5ab442f16789c243648d07b4ec8e2b29 > > This also uncovers problems when building with newer GCC versions which > "helpfully optimize" common malloc/free operations that cause incorrect > code to be generated in some configurations of the library. > > -- > -keith I took a brief look and I'm just wondering why you don't mention renaming the functions and dropping the wrapper macros in the commit message. As for interest, we may want to ask the ARM guys since they provided the code originally. Cc'ing Szabolcs as well as Joey, the original author. Corinna
diff --git a/newlib/libc/stdlib/nano-mallocr.c b/newlib/libc/stdlib/nano-mallocr.c index cef23977e..83234c618 100644 --- a/newlib/libc/stdlib/nano-mallocr.c +++ b/newlib/libc/stdlib/nano-mallocr.c @@ -476,10 +476,8 @@ void * nano_realloc(RARG void * ptr, malloc_size_t size) return NULL; } - /* TODO: There is chance to shrink the chunk if newly requested - * size is much small */ old_size = nano_malloc_usable_size(RCALL ptr); - if (old_size >= size) + if (size <= old_size && (old_size >> 1) < size) return ptr; mem = nano_malloc(RCALL size);
This reduces memory usage when reallocating objects much smaller. Signed-off-by: Keith Packard <keithp@keithp.com> --- newlib/libc/stdlib/nano-mallocr.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) -- 2.28.0