[postgis-devel] in_gml

[Paul Ramsey]

I apologize, that was the worst "bug report" ever. And frankly because
of the nebulous nature of the issues, it's more of a "warning, future
issue" report. So, here's what I'm seeing...

In the in_gml regression, on *one* OS/X machine (but not another that
is one the same versions of OS and compiler) I'm seeing failures in
xlink_1 and xlink_11. I don't think this has anything to do with the
code in xlink, I think I'm seeing a transient failure due to some
other bad memory issues.

I think the final solution will be pulling the in_* code into
liblwgeom where we can properly valgrind it in isolation.

On Sat, Nov 20, 2010 at 12:09 AM, Olivier Courtin
<olivier.courtin at oslandia.com> wrote:
>
> On Nov 19, 2010, at 11:34 PM, Paul Ramsey wrote:
>
> Hi Paul,
>
>> I feel like in_gml is a little ... flaky? Delicate? Right now hudson
>> is regressing with tests that pass while my OSX machine is failing a
>> couple of the xlink tests. It's all very hard to understand.
>
> Humm could you be a bit more precise ?
> Never met such problem on regression test on it
>
>> There's
>> also some odd code in there cloning pointarrays for no obvious reason
>
> There's ptarray_clone usage to copy point from dynamic point array
> to LWGEOM point array.

Here's the note
http://trac.osgeo.org/postgis/browser/trunk/postgis/lwgeom_in_gml.c#L646
but actually I think it's one *I* put in there because the clone made
no sense to me as I was working through the add point code.

> Is there now with your new point array API a way to add point without
> having to use dynamic point array ?

Yes, ptarray_append_point and ptarray_insert_point. These are good
replacements for old dynptarray code (and that's already been done)
but there's also ptarray_addPoint, which is more dangerous. The new
functions act a lot like the dynptarray code, in that they manage the
memory area on an existing ptarray. The old ptarray_addPoint function
allocates a whole new array to add the point to, so the memory
lifecycle is quite different. For writing net new code this should not
be a problem, but retrofitting existing code we have to be quite
careful.

I have recently realized that the new ptarray code is not very
compatible with the old WKT and WKB parsers, since the old parsers
allocate one big chunk of memory for a serialized form, then point
into it, which means when you try to reallocate the
serialized_pointlist in a ptarray to fit more stuff in, things fail.
The new parsers allocate separate chunks for each ptarray, which means
they are individually manageable.

Anyhow, I have to track down the issues Regina has found first, then I
should look at flipping the parsers and emitters over to the new code.
Everything is becoming quite interdependent. In fact, your FLAGS refit
is now pretty key, since it will allow me to flag ptarrays that are
created on top of memory they don't own as READ-ONLY, so we can avoid
nastiness.

Incidentally, you'll note there's a liblwgeom_internal.h header file
now where I'm trying to move truly internal function signatures.

This whole process has also led me back into the ./postgis directory
where I'm finding a surprising amount of core code that has to be
ported back into liblwgeom :)

Anyhow, I'm looking forward to your FLAGS work going in, it will make
it easier to complete the ptarray API in a rational way.

If, while you're at it, you can change lwgeom->SRID to lwgeom->srid,
that would warm my pedant's heart. I hate having that attribute upper
case while all the others are lower case.

Are you changing over the bbox (from BOX2DFLOAT4 to GBOX) in this pass
too? If not, that should be your next goal, IMO.

P.

>
>> and a note that it's being done to dodge some memory issue.
>
> Don't get you on this point,
> What do you refer to ?
>
> --
> Olivier
>
> _______________________________________________
> postgis-devel mailing list
> postgis-devel at postgis.refractions.net
> http://postgis.refractions.net/mailman/listinfo/postgis-devel
>