Re: re-iterating a previous idea: variable length arrays
Geoffrey Biggs <g.biggs <at> auckland.ac.nz>
2007-02-04 08:05:03 GMT
Following on from yesterday's messy (and sometimes incorrect)
stream-of-conciousness, I've done some more work with this. I currently
have a system where I can define a dynamically allocated array in an
interface. It allocates memory automatically when unpacking messages.
All other allocation and deletion needs to be managed manually at the
moment.
It's still subject to a few limitations:
- There's a memory leak when drivers send a dynamic array. We'll get to
that in a minute.
- Dynamic arrays cannot be in embedded structures contained within a
message. For example, if you have an array of structures as part of the
message, those structures cannot contain dynamically allocated arrays.
This is because of the way memory allocation is handled for unpacking
received messages. The xdr filter function called for the embedded
structure type doesn't know if it's encoding or decoding, and so doesn't
know if it needs to allocate memory or not. I'm not sure how to get
around this problem yet.
- When an array is encoded with XDR, it seems to encode the pointer into
the message as well, even though this is rather useless. It results in 4
extra bytes being sent. I'm not sure why this is happening, perhaps
someone more familiar with XDR knows.
It's pretty nasty at the moment. I've attached the necessary
modifications to playerxdrgen.py as a patch for people to try out and
improve.
The main issue is, of course, when memory gets allocated and deleted.
After a bit of thought, I've come up with the following occasions when
it will need to happen (if I've missed any, let me know):
A. When a message goes from client to driver:
1 - Client allocates local copy (handled by client writer).
2 - Client deletes local copy (because client is single-threaded for
writing to server, no conflict with needing to wait till message gets
packed).
3 - Unpacking function on server allocates (handled automatically).
4 - Somewhere on the server, once message has been handled, array needs
to be deleted (handled by driver writer?).
B. When a message goes from driver to client:
1 - Driver allocates local copy (handled by driver writer)
2 - Memory allocated by driver needs to be deleted after message is
packed. Due to threading, message may not be packed as soon as Publish
is called, so cannot delete after Publish returns (memory leak!).
3 - Unpacking function on client allocates (handled automatically).
4 - Client library deletes after handling message, or passes ownership
on via returning to client program (handled either by client lib dev or
client program dev).
The problematic ones are the deletes:
B.4 could be problematic if other transport layers are introduces which
handle dynamic arrays better and so don't need the client library to
delete manually.
B.2 is the worst. Because of threading issues, the driver can't free the
memory itself without risking a race condition, nor can it reuse that
memory. The playertcp can't free it because it doesn't know how to (a
message body is opaque to it, and, as far as it knows, static). What
should probably happen here is that when Publish is called a deep copy
of the message structure is made instead of a shallow copy (which works
fine so far because we haven't had dynamically allocated memory to worry
about), but because the inside of messages isn't known, this is difficult.
A.4 shouldn't be too bad, but it's not that nice for drivers to have to
free memory allocated elsewhere once they've handled a message.
One possible way to manage all this deleting is created a
FinishMessage() function or similar that, when called, will clean up a
message after it's been used (either packed or handled). This function
will need to know how to clean up each message, so a customised function
for each message is necessary. However, rather than writing these, we
could automatically generate them similar to the packing functions, and
use a function table to look them up in FinishMessage() and call the
appropriate one. Messages without any dynamically allocated memory would
have an empty function, and if we inlined these they would disappear
(optimising compilers should remove empty functions anyway). The
function table lookup could be considered wasteful, though, when the
bulk of messages don't need cleaning up. Auto-generation could also be
used to create functions to handle deep copies of messages made when
they get pushed onto message queues.
Geoff
Geoffrey Biggs wrote:
> I started having a look at variable length arrays today since it seemed
> like a good way to learn how the XDR parts of Player work. I'm using the
> method Brian suggested on the task manager page for it. So far I haven't
> done much, just got the playerxdrgen.py script to recognise pointers and
> find the _count variable.
>
> The main implementation issue, and the one that's always killed
> discussion in the past, seems to be where memory is
> allocated/deallocated. My understanding of the way the XDR stuff works
> is that a buffer is allocated for the packed data just before it is
> packed and sent over the transport. It's easy enough to allocate this
> buffer correctly for the size of a variable length array. So the issue
> is driver and client writers needing to allocate/deallocate.
>
> Drivers and clients would probably have to allocate the array themselves
> before publishing, sending requests, etc, and this is where memory leak
> issues start to appear. I can imagine that most drivers using an
> interface with a variable length array would be sending regular large
> chunks of data of a similar size (such as a video frame) and in this
> case could reuse the array for each new message they publish. Same for
> clients making regular requests. Otherwise they'll have to allocate and
> remember to deallocate the array in their local copy of the message
> themselves.
>
> Because drivers/clients may want to reuse a message structure, including
> any variable length arrays, after publishing a message, I don't think we
> can put deallocation of the array into the XDR packing function so that
> it gets deleted when the message is packed. But we could possibly add a
> check for if the array pointer is NULL for unpacking, and have the
> function allocate necessary memory in this case. Although in the case of
> clients, the buffer is already allocated to be max message size so no
> need to allocate anything there, and if the buffer to unpack into isn't
> zeroed the packing function may not allocate as necessary. Since
> structures lack intelligence we can't add constructors/destructors
> without forcing the entire interface system into C++, which probably
> isn't desirable.
>
> That's about as far as I've got in thinking about how to do this so far.
> Does anyone else have any comments?
>
> Geoff
--
Robotics research group, University of Auckland
http://www.ece.auckland.ac.nz/~gbig005/
Index: playerxdrgen.py
===================================================================
RCS file: /cvsroot/playerstage/code/player/libplayerxdr/playerxdrgen.py,v
retrieving revision 1.8
diff -u -r1.8 playerxdrgen.py
--- playerxdrgen.py 7 Aug 2006 14:17:59 -0000 1.8
+++ playerxdrgen.py 4 Feb 2007 08:00:27 -0000
<at> <at> -1,6 +1,6 <at> <at>
#!/usr/bin/env python
-#TODO:
+#TODO:
# - Add an option to specify whether we're building libplayerxdr (whose
# header gets installed for general use, has copyright boilerplate,
# etc.) or a user XDR lib
<at> <at> -18,7 +18,7 <at> <at>
sys.exit(-1)
distro = 0
-
+
idx = 1
if len(sys.argv) == 5:
if sys.argv[idx] == '-distro':
<at> <at> -27,7 +27,7 <at> <at>
else:
print USAGE
sys.exit(-1)
-
+
infilename = sys.argv[idx]
idx += 1
sourcefilename = sys.argv[idx]
<at> <at> -50,7 +50,7 <at> <at>
pattern = re.compile('/\*.*?\*/', re.MULTILINE | re.DOTALL)
instream = pattern.sub('', instream)
- # strip blank lines
+ # strip blank lines
pattern = re.compile('^\s*?\n', re.MULTILINE)
instream = pattern.sub('', instream)
<at> <at> -58,7 +58,7 <at> <at>
pattern = re.compile('typedef\s+struct\s+player_\w+[^}]+\}[^;]+',
re.MULTILINE)
structs = pattern.findall(instream)
-
+
print 'Found ' + `len(structs)` + ' struct(s)'
if distro:
<at> <at> -103,6 +103,8 <at> <at>
variablepattern = re.compile('\s*([^,;]+?)\s*[,;]')
#arraypattern = re.compile('\[\s*(\w*?)\s*\]')
arraypattern = re.compile('\[(.*?)\]')
+# pointerpattern = re.compile('[A-Za-z0-9_]+\*|\*[A-Za-z0-9_]+')
+ pointerpattern = re.compile('\*')
for s in structs:
# extract prefix for packing function name and type of struct
<at> <at> -124,9 +126,9 <at> <at>
for i in range(0,2):
if i == 0:
- headerfile.write('int xdr_' + typename + '(XDR* xdrs, ' + typename +
+ headerfile.write('int xdr_' + typename + '(XDR* xdrs, ' + typename +
'* msg);\n')
- sourcefile.write('int\nxdr_' + typename + '(XDR* xdrs, ' + typename +
+ sourcefile.write('int\nxdr_' + typename + '(XDR* xdrs, ' + typename +
'* msg)\n{\n')
else:
headerfile.write('int ' + prefix + '_pack(void* buf, size_t buflen, ' +
<at> <at> -140,7 +142,7 <at> <at>
sourcefile.write(' xdrmem_create(&xdrs, buf, buflen, op);\n')
varlist = []
-
+
# separate the variable declarations
decls = declpattern.findall(varpart[0])
for dstring in decls:
<at> <at> -148,10 +150,14 <at> <at>
type = typepattern.findall(dstring)[0]
dstring = typepattern.sub('', dstring, 1)
vars = variablepattern.findall(dstring)
-
+
+ # Check for pointer in type part
+ numstars = len (pointerpattern.findall (type))
+ if numstars > 0:
+ type = type[:-1 * numstars]
# Do some name mangling for common types
- if type == 'long long':
- xdr_proc = 'xdr_longlong_t'
+ if type == 'long long':
+ xdr_proc = 'xdr_longlong_t'
elif type == 'int64_t':
xdr_proc = 'xdr_longlong_t'
elif type == 'uint64_t':
<at> <at> -173,92 +179,147 <at> <at>
else:
# rely on a previous declaration of an xdr_ proc for this type
xdr_proc = 'xdr_' + type
-
+
# iterate through each variable
for varstring in vars:
# is it an array or a scalar?
+ array = False
arraysize = arraypattern.findall(varstring)
+ pointersize = pointerpattern.findall(varstring)
if len(arraysize) > 0:
- arraysize = arraysize[0]
- varstring = arraypattern.sub('', varstring)
+ array = True
+ arraysize = arraysize[0]
+ varstring = arraypattern.sub('', varstring)
+ elif len(pointersize) > 0 and numstars > 0:
+ raise 'Illegal pointer declaration in struct\n' + s
+ elif len(pointersize) > 0 or numstars > 0:
+ array = True
+ arraysize = ''
+ varstring = pointerpattern.sub('', varstring)
+
+ #if len(arraysize) > 0:
+ #arraysize = arraysize[0]
+ #varstring = arraypattern.sub('', varstring)
+ #pointervar = varstring + '_p'
+ #countvar = varstring + '_count'
+ if array:
pointervar = varstring + '_p'
countvar = varstring + '_count'
-
- # Was a _count variable declared? If so, we'll encode as a
- # variable-length array (with xdr_array); otherwise we'll
- # do it fixed-length (with xdr_vector). xdr_array is picky; we
- # have to declare a pointer to the array, then pass in the
- # address of this pointer. Passing the address of the array
- # does NOT work.
- if countvar in varlist:
-
+ if arraysize == '': # Handle a dynamically allocated array
+ # Check for a matching count variable, because compulsory for dynamic arrays
+ if countvar not in varlist:
+ raise 'Missing count var "' + countvar + '" in\n' + s
+ # First put in a check to see if unpacking, and if so allocate memory for the array
+ # !!!!! Only done for packing function, not XDR filter function! Need to figure out what
+ # to do in the filter function, where can't tell if encoding or decoding
+ if i == 1:
+ sourcefile.write(' if(op == PLAYERXDR_DECODE)\n {\n')
+ sourcefile.write(' printf ("mallocing %d bytes in decode\\n", msg->' + countvar + '*sizeof(' + type + '));\n')
+ sourcefile.write(' if((msg->' + varstring + ' = malloc(msg->' + countvar + '*sizeof(' + type + '))) ==
NULL)\n return(-1);\n')
+ #sourcefile.write(' memset(msg->' + varstring + ', 0, msg->' + countvar + '*sizeof(' + type + '));\n')
+ sourcefile.write(' }\n')
+ # Write the lines to (un)pack/convert
# Is it an array of bytes? If so, then we'll encode
# it a packed opaque bytestring, rather than an array of 4-byte-aligned
# chars.
if xdr_proc == 'xdr_u_char' or xdr_proc == 'xdr_char':
if i == 0:
sourcefile.write(' {\n')
- sourcefile.write(' ' + type + '* ' + pointervar +
- ' = msg->' + varstring + ';\n')
- sourcefile.write(' if(xdr_bytes(xdrs, (char**)&' + pointervar +
- ', &msg->' + countvar +
- ', ' + arraysize + ') != 1)\n return(0);\n')
+ sourcefile.write(' ' + type + '* ' + pointervar + ' = msg->' + varstring + ';\n')
+ sourcefile.write(' if(xdr_bytes(xdrs, (char**)&' + pointervar + ', &msg->' + countvar + ', msg->' +
countvar + ') != 1)\n return(0);\n')
sourcefile.write(' }\n')
else:
sourcefile.write(' {\n')
- sourcefile.write(' ' + type + '* ' + pointervar +
- ' = msg->' + varstring + ';\n')
- sourcefile.write(' if(xdr_bytes(&xdrs, (char**)&' + pointervar +
- ', &msg->' + countvar +
- ', ' + arraysize + ') != 1)\n return(-1);\n')
+ sourcefile.write(' ' + type + '* ' + pointervar + ' = msg->' + varstring + ';\n')
+ sourcefile.write(' if(xdr_bytes(&xdrs, (char**)&' + pointervar + ', &msg->' + countvar + ', msg->' +
countvar + ') != 1)\n return(-1);\n')
sourcefile.write(' }\n')
else:
if i == 0:
sourcefile.write(' {\n')
- sourcefile.write(' ' + type + '* ' + pointervar +
- ' = msg->' + varstring + ';\n')
- sourcefile.write(' if(xdr_array(xdrs, (char**)&' + pointervar +
- ', &msg->' + countvar +
- ', ' + arraysize + ', sizeof(' + type + '), (xdrproc_t)' +
- xdr_proc + ') != 1)\n return(0);\n')
+ sourcefile.write(' ' + type + '* ' + pointervar + ' = msg->' + varstring + ';\n')
+ sourcefile.write(' if(xdr_array(xdrs, (char**)&' + pointervar + ', &msg->' + countvar + ', msg->' +
countvar +', sizeof(' + type + '), (xdrproc_t)' + xdr_proc + ') != 1)\n return(0);\n')
sourcefile.write(' }\n')
else:
sourcefile.write(' {\n')
- sourcefile.write(' ' + type + '* ' + pointervar +
- ' = msg->' + varstring + ';\n')
- sourcefile.write(' if(xdr_array(&xdrs, (char**)&' + pointervar +
- ', &msg->' + countvar +
- ', ' + arraysize + ', sizeof(' + type + '), (xdrproc_t)' +
- xdr_proc + ') != 1)\n return(-1);\n')
+ sourcefile.write(' ' + type + '* ' + pointervar + ' = msg->' + varstring + ';\n')
+ sourcefile.write(' if(xdr_array(&xdrs, (char**)&' + pointervar + ', &msg->' + countvar + ', msg->' +
countvar +', sizeof(' + type + '), (xdrproc_t)' + xdr_proc + ') != 1)\n return(-1);\n')
sourcefile.write(' }\n')
- else:
- # Is it an array of bytes? If so, then we'll encode
- # it a packed opaque bytestring, rather than an array of 4-byte-aligned
- # chars.
- if xdr_proc == 'xdr_u_char' or xdr_proc == 'xdr_char':
- if i == 0:
- sourcefile.write(' if(xdr_opaque(xdrs, (char*)&msg->' +
- varstring + ', ' + arraysize + ') != 1)\n return(0);\n')
+ else: # Handle a static array
+ # Was a _count variable declared? If so, we'll encode as a
+ # variable-length array (with xdr_array); otherwise we'll
+ # do it fixed-length (with xdr_vector). xdr_array is picky; we
+ # have to declare a pointer to the array, then pass in the
+ # address of this pointer. Passing the address of the array
+ # does NOT work.
+ if countvar in varlist:
+
+ # Is it an array of bytes? If so, then we'll encode
+ # it a packed opaque bytestring, rather than an array of 4-byte-aligned
+ # chars.
+ if xdr_proc == 'xdr_u_char' or xdr_proc == 'xdr_char':
+ if i == 0:
+ sourcefile.write(' {\n')
+ sourcefile.write(' ' + type + '* ' + pointervar +
+ ' = msg->' + varstring + ';\n')
+ sourcefile.write(' if(xdr_bytes(xdrs, (char**)&' + pointervar +
+ ', &msg->' + countvar +
+ ', ' + arraysize + ') != 1)\n return(0);\n')
+ sourcefile.write(' }\n')
+ else:
+ sourcefile.write(' {\n')
+ sourcefile.write(' ' + type + '* ' + pointervar +
+ ' = msg->' + varstring + ';\n')
+ sourcefile.write(' if(xdr_bytes(&xdrs, (char**)&' + pointervar +
+ ', &msg->' + countvar +
+ ', ' + arraysize + ') != 1)\n return(-1);\n')
+ sourcefile.write(' }\n')
else:
- sourcefile.write(' if(xdr_opaque(&xdrs, (char*)&msg->' +
- varstring + ', ' + arraysize + ') != 1)\n return(-1);\n')
+ if i == 0:
+ sourcefile.write(' {\n')
+ sourcefile.write(' ' + type + '* ' + pointervar +
+ ' = msg->' + varstring + ';\n')
+ sourcefile.write(' if(xdr_array(xdrs, (char**)&' + pointervar +
+ ', &msg->' + countvar +
+ ', ' + arraysize + ', sizeof(' + type + '), (xdrproc_t)' +
+ xdr_proc + ') != 1)\n return(0);\n')
+ sourcefile.write(' }\n')
+ else:
+ sourcefile.write(' {\n')
+ sourcefile.write(' ' + type + '* ' + pointervar +
+ ' = msg->' + varstring + ';\n')
+ sourcefile.write(' if(xdr_array(&xdrs, (char**)&' + pointervar +
+ ', &msg->' + countvar +
+ ', ' + arraysize + ', sizeof(' + type + '), (xdrproc_t)' +
+ xdr_proc + ') != 1)\n return(-1);\n')
+ sourcefile.write(' }\n')
else:
- if i == 0:
- sourcefile.write(' if(xdr_vector(xdrs, (char*)&msg->' +
- varstring + ', ' + arraysize +
- ', sizeof(' + type + '), (xdrproc_t)' +
- xdr_proc + ') != 1)\n return(0);\n')
+ # Is it an array of bytes? If so, then we'll encode
+ # it a packed opaque bytestring, rather than an array of 4-byte-aligned
+ # chars.
+ if xdr_proc == 'xdr_u_char' or xdr_proc == 'xdr_char':
+ if i == 0:
+ sourcefile.write(' if(xdr_opaque(xdrs, (char*)&msg->' +
+ varstring + ', ' + arraysize + ') != 1)\n return(0);\n')
+ else:
+ sourcefile.write(' if(xdr_opaque(&xdrs, (char*)&msg->' +
+ varstring + ', ' + arraysize + ') != 1)\n return(-1);\n')
else:
- sourcefile.write(' if(xdr_vector(&xdrs, (char*)&msg->' +
- varstring + ', ' + arraysize +
- ', sizeof(' + type + '), (xdrproc_t)' +
- xdr_proc + ') != 1)\n return(-1);\n')
+ if i == 0:
+ sourcefile.write(' if(xdr_vector(xdrs, (char*)&msg->' +
+ varstring + ', ' + arraysize +
+ ', sizeof(' + type + '), (xdrproc_t)' +
+ xdr_proc + ') != 1)\n return(0);\n')
+ else:
+ sourcefile.write(' if(xdr_vector(&xdrs, (char*)&msg->' +
+ varstring + ', ' + arraysize +
+ ', sizeof(' + type + '), (xdrproc_t)' +
+ xdr_proc + ') != 1)\n return(-1);\n')
else:
if i == 0:
- sourcefile.write(' if(' + xdr_proc + '(xdrs,&msg->' +
+ sourcefile.write(' if(' + xdr_proc + '(xdrs,&msg->' +
varstring + ') != 1)\n return(0);\n')
else:
- sourcefile.write(' if(' + xdr_proc + '(&xdrs,&msg->' +
+ sourcefile.write(' if(' + xdr_proc + '(&xdrs,&msg->' +
varstring + ') != 1)\n return(-1);\n')
varlist.append(varstring)
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Playerstage-developers mailing list
Playerstage-developers <at> lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/playerstage-developers
I'm not having any problems yet. I haven't actually gotten around to a complete test, I was just getting set up and got a warning that laser_range_max wasn't being used... I ran amcl with a map I generated, and it seems to mostly work, but I wanted to visualize it to make certain. Is playernav the best way to do this? Or should I configure with --enable-rtkgui? --brendan On 1/31/07, Brian Gerkey <brian <at> gerkey.org> wrote: > > On Jan 31, 2007, at 2:24 PM, Brendan Burns wrote: > > > Hey folks, > > I'm trying to get amcl working with a urg laser. I noticed that the > > laser_range_max variable is not loaded from the config file. Adding: > > > > this->range_max = cf->ReadLength(section, "laser_range_max", 8.192); > > > > to amcl.cc should fix things... > > Actually, that option was removed from amcl when we added the > max_range field to the laser data packet. Now the laser reports its > own maximum range, so you don't need to specify it separately. > > The urglaser driver should properly report its max range, and I would > expect amcl to work fine. What sort of problems are you having?
RSS Feed