getPoint_internal dangerous?

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

getPoint_internal dangerous?

Björn Harrtell
I want to mutate one axis of the points in a POINTARRAY in the most efficient way.

Something similar to ptarray_longitude_shift, see:

I see memcpy being used there and other places and afaik it has something to do with fears of struct misalignment.

Does this mean I should most definitely not do this? :)

POINT2D *p = (POINT2D *) getPoint_internal(pa, 0);
p->y = extent - p->y;

Regards,
Björn

_______________________________________________
postgis-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: getPoint_internal dangerous?

Paul Ramsey-3
It's not a struct alignment thing (anymore) but there is a danger in mutating the point array storage directly, because in the database context it's not clear who owns it, and we've had problems in the past w/ functions that directly write into the database storage. If you're going to be changing data, the safest thing is always to take a copy at the start.

P.

On Mon, Mar 6, 2017 at 12:25 PM, Björn Harrtell <[hidden email]> wrote:
I want to mutate one axis of the points in a POINTARRAY in the most efficient way.

Something similar to ptarray_longitude_shift, see:

I see memcpy being used there and other places and afaik it has something to do with fears of struct misalignment.

Does this mean I should most definitely not do this? :)

POINT2D *p = (POINT2D *) getPoint_internal(pa, 0);
p->y = extent - p->y;

Regards,
Björn

_______________________________________________
postgis-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/postgis-devel


_______________________________________________
postgis-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: getPoint_internal dangerous?

Björn Harrtell
Ok I see, I didn't realize I was this close to the database storage/cache. 

But then I fail to see how a memcpy straight into the pointer of a getPoint_internal can be any better? This is done all over the place.

/Björn

2017-03-06 21:28 GMT+01:00 Paul Ramsey <[hidden email]>:
It's not a struct alignment thing (anymore) but there is a danger in mutating the point array storage directly, because in the database context it's not clear who owns it, and we've had problems in the past w/ functions that directly write into the database storage. If you're going to be changing data, the safest thing is always to take a copy at the start.

P.

On Mon, Mar 6, 2017 at 12:25 PM, Björn Harrtell <[hidden email]> wrote:
I want to mutate one axis of the points in a POINTARRAY in the most efficient way.

Something similar to ptarray_longitude_shift, see:

I see memcpy being used there and other places and afaik it has something to do with fears of struct misalignment.

Does this mean I should most definitely not do this? :)

POINT2D *p = (POINT2D *) getPoint_internal(pa, 0);
p->y = extent - p->y;

Regards,
Björn

_______________________________________________
postgis-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/postgis-devel



_______________________________________________
postgis-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: getPoint_internal dangerous?

Paul Ramsey-3
It's no better, do feel free to write directly into the pointer. Just make sure you aren't doing so w/i the context of an lwgeom that has been built directly on top of database-allocated memory. Namely, writing into this lwgeom is dangerous:

GSERIALIZED *gser = PG_GETARG_GSERIALIZED_P(0);
LWGEOM *lwg = lwgeom_from_gserialized(gser);

Writing into this one (lwgc) is OK:

GSERIALIZED *gser = PG_GETARG_GSERIALIZED_P(0);
LWGEOM *lwg = lwgeom_from_gserialized(gser);
LWGEOM *lwgc = lwgeom_clone_deep(lwg);

The first lwgeom is build with POINTARRAYs that point directly into the GSERIALIZED buffer. The second has POINTARRAYs that own their own buffers.

P.


On Mon, Mar 6, 2017 at 12:35 PM, Björn Harrtell <[hidden email]> wrote:
Ok I see, I didn't realize I was this close to the database storage/cache. 

But then I fail to see how a memcpy straight into the pointer of a getPoint_internal can be any better? This is done all over the place.

/Björn

2017-03-06 21:28 GMT+01:00 Paul Ramsey <[hidden email]>:
It's not a struct alignment thing (anymore) but there is a danger in mutating the point array storage directly, because in the database context it's not clear who owns it, and we've had problems in the past w/ functions that directly write into the database storage. If you're going to be changing data, the safest thing is always to take a copy at the start.

P.

On Mon, Mar 6, 2017 at 12:25 PM, Björn Harrtell <[hidden email]> wrote:
I want to mutate one axis of the points in a POINTARRAY in the most efficient way.

Something similar to ptarray_longitude_shift, see:

I see memcpy being used there and other places and afaik it has something to do with fears of struct misalignment.

Does this mean I should most definitely not do this? :)

POINT2D *p = (POINT2D *) getPoint_internal(pa, 0);
p->y = extent - p->y;

Regards,
Björn

_______________________________________________
postgis-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/postgis-devel




_______________________________________________
postgis-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: getPoint_internal dangerous?

Björn Harrtell
Thanks Paul, that makes it quite clear.

/Björn

2017-03-06 22:16 GMT+01:00 Paul Ramsey <[hidden email]>:
It's no better, do feel free to write directly into the pointer. Just make sure you aren't doing so w/i the context of an lwgeom that has been built directly on top of database-allocated memory. Namely, writing into this lwgeom is dangerous:

GSERIALIZED *gser = PG_GETARG_GSERIALIZED_P(0);
LWGEOM *lwg = lwgeom_from_gserialized(gser);

Writing into this one (lwgc) is OK:

GSERIALIZED *gser = PG_GETARG_GSERIALIZED_P(0);
LWGEOM *lwg = lwgeom_from_gserialized(gser);
LWGEOM *lwgc = lwgeom_clone_deep(lwg);

The first lwgeom is build with POINTARRAYs that point directly into the GSERIALIZED buffer. The second has POINTARRAYs that own their own buffers.

P.


On Mon, Mar 6, 2017 at 12:35 PM, Björn Harrtell <[hidden email]> wrote:
Ok I see, I didn't realize I was this close to the database storage/cache. 

But then I fail to see how a memcpy straight into the pointer of a getPoint_internal can be any better? This is done all over the place.

/Björn

2017-03-06 21:28 GMT+01:00 Paul Ramsey <[hidden email]>:
It's not a struct alignment thing (anymore) but there is a danger in mutating the point array storage directly, because in the database context it's not clear who owns it, and we've had problems in the past w/ functions that directly write into the database storage. If you're going to be changing data, the safest thing is always to take a copy at the start.

P.

On Mon, Mar 6, 2017 at 12:25 PM, Björn Harrtell <[hidden email]> wrote:
I want to mutate one axis of the points in a POINTARRAY in the most efficient way.

Something similar to ptarray_longitude_shift, see:

I see memcpy being used there and other places and afaik it has something to do with fears of struct misalignment.

Does this mean I should most definitely not do this? :)

POINT2D *p = (POINT2D *) getPoint_internal(pa, 0);
p->y = extent - p->y;

Regards,
Björn

_______________________________________________
postgis-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/postgis-devel





_______________________________________________
postgis-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/postgis-devel
Loading...