[Monetdb-developers] @= atomtostr memory usage

Stefan de Konink stefan at konink.de
Thu Feb 11 05:00:17 CET 2010


Hi,


While creating some new code and seeing unexpected, but interesting,
behavior I stumbled upon the @= atomtostr code, some simple questions
regarding this.


It seems that the memory requested in @= atommem is rather pessimistic. It
seems that for a regular integer 24 characters are allocated. Now since
this integer seems to go up to 2^31 (10 chars) + sign + \0 (2 chars), why
is the double amount needed if a per type approach is available in MAL?

The actual toStr code finishes with a strlen, now this is nice behavior,
but if we now know what the length of the string actually is, why not
shoot a GDKrealloc on it, and make the *len the actual size?


Regarding to memory management and memory function calling, the code isn't
very efficient. Effectively all the functions seem to introduce their own
buffers. If we look from a perspective of for example strConcat, the value
is copied several times from serveral position. We could overcome this
bahavior if we would return a maximum length for a type, the
concat function would add the length of the second string alloc this
buffer, forwards the pointer to the typical atomToStr call, and instead of
allocing a new string, using the prealloced buffer.
(...and yes, doing a GDKrealloc after that function wouldn't be good.)


I took the liberty to come up with a patch for the second issue;
Since snprintf will return the amount of bytes written, and since we have
established that there is no numeric type overflowing the string space, we
can ommit the strlen, use the amount of bytes + 1 from snprintf, realloc,
and return the actual amount of bytes representing the string.

Catches;
1) Old implementation resulted bestcase in an off by one return vs len
2) Is there any code dependent on the *len or that length will include the
\0 value?


If there are no objections, I'll check in that patch later today in head.
>From benchmarking point of view, it might be worthwhile to test with and
without the realloc.


Stefan
-------------- next part --------------
Index: gdk/gdk_atoms.mx
===================================================================
RCS file: /cvsroot/monetdb/MonetDB/src/gdk/gdk_atoms.mx,v
retrieving revision 1.175
diff -u -r1.175 gdk_atoms.mx
--- gdk/gdk_atoms.mx	28 Jan 2010 12:51:09 -0000	1.175
+++ gdk/gdk_atoms.mx	11 Feb 2010 03:43:24 -0000
@@ -1105,8 +1105,9 @@
 		strcpy(*dst, "nil");
 		return 3;
 	}
-	snprintf(*dst, *len, @2, (@3) *src);
-	return (int) strlen(*dst);
+	*len = snprintf(*dst, *len, @2, (@3) *src);
+	GDKrealloc(*dst, (*len + 1));
+	return *len;
 }
 @c
 #define num08(x)	((x) >= '0' && (x) <= '7')


More information about the developers-list mailing list