GetGeometryRef in scripts

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

GetGeometryRef in scripts

Kevin Ruland

Frank,

Hopefully you know where I'm comming from right now....  memory control
in script land.  I am unfortunately confused a little.  There are some
methods which I'd like to have clarification on.

Feature::GetGeometryRef
Feature::SetGeometry
Feature::SetGeometryDirectly

Geometry::GetGeometryRef
Geometry::AddGeometryDirectly
Geometry::AddGeometry

I roughly understand the difference between SetGeometry and
SetGeometryDirectly in the latter assumes ownership of the Geometry*
argument and the former executes some clone process.

Is this additional complexity required in scripts?  Can we get by with
only SetGeometry?

What is Geometry::GetGeometryRef()?  How would it be used?

Charlie indicates the Geometry object returned by
Feature::GetGeometryRef is owned by the the Feature object.  Would it be
ok to change this from it's current implementation:

return OGR_F_GetGeometryRef(self)

To a method which returns a clone whos memory is managed by the script,

OGRGeometryH g = OGR_F_GetGeometryRef(self)
return OGR_G_Clone( g );

What about the two different versions of AddGeometry?

Thanks a bunch.

kevin
_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Frank Warmerdam
On 9/20/05, Kevin Ruland <[hidden email]> wrote:

>
> Frank,
>
> Hopefully you know where I'm comming from right now....  memory control
> in script land.  I am unfortunately confused a little.  There are some
> methods which I'd like to have clarification on.
>
> Feature::GetGeometryRef
> Feature::SetGeometry
> Feature::SetGeometryDirectly
>
> Geometry::GetGeometryRef
> Geometry::AddGeometryDirectly
> Geometry::AddGeometry
>
> I roughly understand the difference between SetGeometry and
> SetGeometryDirectly in the latter assumes ownership of the Geometry*
> argument and the former executes some clone process.

Kevin,

Right.

> Is this additional complexity required in scripts?  Can we get by with
> only SetGeometry?

That is potentially sacrificing significant performance, but if you
can't see a practical way to void it, then so be it.  

> What is Geometry::GetGeometryRef()?  How would it be used?

This method returns a subgeometry if the geometry is a container.
 

> Charlie indicates the Geometry object returned by
> Feature::GetGeometryRef is owned by the the Feature object.  Would it be
> ok to change this from it's current implementation:
>
> return OGR_F_GetGeometryRef(self)
>
> To a method which returns a clone whos memory is managed by the script,
>
> OGRGeometryH g = OGR_F_GetGeometryRef(self)
> return OGR_G_Clone( g );

With the understanding that you are potentially adding quite
a bit of runtime cost, I suppose this would be acceptable, but
if practical I would prefer to keep the current approach.
 
> What about the two different versions of AddGeometry?

Likewise.  We could settle for just AddGeometry(), but there
is a non-trivial cost in some applications.

Scripting languages like python are not particularly slow,
and it is quite practical to process large volumns of data
with it.  But if you force lots of extra object copying you
will make it difficult to write efficient python scripts for
some applications.  The same would apply to other languages
I would imagine.

In the old python interface we carried around a flag indicating
whether a given object reference "owned" that object or
not.  At least this was done for stuff like geometries.  Is
there a reason that approach cannot be generalized?  

In the past, I have opted to keep the scripting interface pretty
close to the underlying C++ library for the most part; however,
I am willing to defer to the you and others doing the majority
of the swig interface work if you prefer to emphasise safety
over performance.

Best regards,
--
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, [hidden email]
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Ari Jolma
I think the script writer understands the difference between "a geometry
as a part of something" and "a separate geometry object". The first one
goes away when the something goes away and second one is there as long
as the variable is. In our example:

geometry = dataset.getlayer.getgeometry

The temporary layer is always part of something but the geometry becomes
a separate object because of the assignment. At least in Perl you can
detect this situation and instantiate a new geometry as a copy of the
temporary reference which exists for a while. If you have

dataset.getlayer.getgeometry.do_something_with_a_geometry

Then the do_something_with_a_geometry will work on the temporary reference.

I have lots of functions like this in my Geo::Raster that can be used as
"in-place" or "make-a-copy".

I hope this helps.

Ari


Frank Warmerdam kirjoitti:

>On 9/20/05, Kevin Ruland <[hidden email]> wrote:
>  
>
>>Frank,
>>
>>Hopefully you know where I'm comming from right now....  memory control
>>in script land.  I am unfortunately confused a little.  There are some
>>methods which I'd like to have clarification on.
>>
>>Feature::GetGeometryRef
>>Feature::SetGeometry
>>Feature::SetGeometryDirectly
>>
>>Geometry::GetGeometryRef
>>Geometry::AddGeometryDirectly
>>Geometry::AddGeometry
>>
>>I roughly understand the difference between SetGeometry and
>>SetGeometryDirectly in the latter assumes ownership of the Geometry*
>>argument and the former executes some clone process.
>>    
>>
>
>Kevin,
>
>Right.
>
>  
>
>>Is this additional complexity required in scripts?  Can we get by with
>>only SetGeometry?
>>    
>>
>
>That is potentially sacrificing significant performance, but if you
>can't see a practical way to void it, then so be it.  
>
>  
>
>>What is Geometry::GetGeometryRef()?  How would it be used?
>>    
>>
>
>This method returns a subgeometry if the geometry is a container.
>
>  
>
>>Charlie indicates the Geometry object returned by
>>Feature::GetGeometryRef is owned by the the Feature object.  Would it be
>>ok to change this from it's current implementation:
>>
>>return OGR_F_GetGeometryRef(self)
>>
>>To a method which returns a clone whos memory is managed by the script,
>>
>>OGRGeometryH g = OGR_F_GetGeometryRef(self)
>>return OGR_G_Clone( g );
>>    
>>
>
>With the understanding that you are potentially adding quite
>a bit of runtime cost, I suppose this would be acceptable, but
>if practical I would prefer to keep the current approach.
>
>  
>
>>What about the two different versions of AddGeometry?
>>    
>>
>
>Likewise.  We could settle for just AddGeometry(), but there
>is a non-trivial cost in some applications.
>
>Scripting languages like python are not particularly slow,
>and it is quite practical to process large volumns of data
>with it.  But if you force lots of extra object copying you
>will make it difficult to write efficient python scripts for
>some applications.  The same would apply to other languages
>I would imagine.
>
>In the old python interface we carried around a flag indicating
>whether a given object reference "owned" that object or
>not.  At least this was done for stuff like geometries.  Is
>there a reason that approach cannot be generalized?  
>
>In the past, I have opted to keep the scripting interface pretty
>close to the underlying C++ library for the most part; however,
>I am willing to defer to the you and others doing the majority
>of the swig interface work if you prefer to emphasise safety
>over performance.
>
>Best regards,
>  
>


--
Prof. Ari Jolma
Kartografia ja Geoinformatiikka / Cartography and Geoinformatics
Teknillinen Korkeakoulu / Helsinki University of Technology
POBox 1200, 02015 TKK, Finland
Email: ari.jolma at tkk.fi URL: http://www.tkk.fi/~jolma

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Kevin Ruland
In reply to this post by Frank Warmerdam
Frank,

You are right, performance is important particularly for scripts.  I for
one am quite guilty of whipping up bad one-off code to spin through
thousands of datasets.  It would be good if these could be made efficient.

I'd like to discuss the potential problems to making life easy for
scripters.  The problem with Geometry is we really have three different
kinds of geometry - those owned by nobody, those owned by Features,
those owned by Geometries. (please correct me)

Feature::GetGeometryRef()
Geometry::GetGeometryRef()

And it's owner is either a Feature or Geometry depend on how you got a
hold of it.  You get Geometries which are not owned by anybody by doing:

Geometry::Clone()

There is probably a pure constructor for Geometry as well.

Let's bundle these handles together like this:

struct GeometryGuy {
  OGRFeatureH parentFeature;
  OGRGeometryH parentGeometry;
  OGRGeometryH me;
}

Now there are a bunch of functions which fall into two categories:

Those which make copies:

Feature::SetGeometry()
Geometry::AddGeometry()

Those which assume ownership:

Feature::SetGeometryDirectly()
Geometry::AddGeometryDirectly()

And all the Geometry member functions which operate correctly
reguardless of ownership.

It seems, if the script bindings operate on the GeometryGuy (instead of
OGRGeometryH), we can drop the distinction between the SetGeometry &
SetGeometryDirectly and have the wrapper function determine which method
to envoke.  Of course, the method is determined by which of the parent*
pointers is populated.

I believe a strategy like this would hide the troubles of passing the
wrong kind of Geometry into the wrong method, and at the same time
provide for maximum efficiency.  Yes there would be code overhead to
provide the safety -- extra tests, more complex implementation of
constructors (Clone etc).

Also instead of using OGRFeatureH, we would have to add some reference
counting for the handle in order to allow for chained calls.

Kevin


_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Frank Warmerdam
On 9/20/05, Kevin Ruland <[hidden email]> wrote:
> It seems, if the script bindings operate on the GeometryGuy (instead of
> OGRGeometryH), we can drop the distinction between the SetGeometry &
> SetGeometryDirectly and have the wrapper function determine which method
> to envoke.  Of course, the method is determined by which of the parent*
> pointers is populated.

Kevin,

I don't really see that this is practical.  Even if we have a "free"
geometry we don't know that we ought to use SetGeometryDirectly()
since we don't know whether the script will continue to use the
geometry reference later.
 
> I believe a strategy like this would hide the troubles of passing the
> wrong kind of Geometry into the wrong method, and at the same time
> provide for maximum efficiency.  Yes there would be code overhead to
> provide the safety -- extra tests, more complex implementation of
> constructors (Clone etc).
 
> Also instead of using OGRFeatureH, we would have to add some reference
> counting for the handle in order to allow for chained calls.

I am depressed by this whole concept of bending over backward
to support chaining.  I seems wrong to do all this backward referencing
of objects and I feel like it will one day make us all sorry.  

However, I leave it in your hands.

Best regards,
--
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, [hidden email]
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Kevin Ruland

>Kevin,
>
>I don't really see that this is practical.  Even if we have a "free"
>geometry we don't know that we ought to use SetGeometryDirectly()
>since we don't know whether the script will continue to use the
>geometry reference later.
>
>  
>

Frank,

Ah another complexity, a single free geometry can only be the argument
of one *Directly call.  This stuff is certainly not easy.

>I am depressed by this whole concept of bending over backward
>to support chaining.  I seems wrong to do all this backward referencing
>of objects and I feel like it will one day make us all sorry.  
>
>  
>
I can't say that I too am depressed ... a better description would be
"frustrated," although I completely sympathize with your feelings.  It
seems the more friendly we try to make it the more complex its going to
become and eventually the bindings will become soooo complex that only I
can maintain them.  In which case you're actually worse off than before :)

The current swig bindings have some short falls but I do believe they
are fairly useable provided the developer follow the guidelines of the
underlying C++ library.  The rules in a nutshell are:

1) you must maintain a handle to the Datasource as long as you are using
objects from it (with some exceptions such as XXX).  If the Datsource is
destroyed, developer pays for all damages.
2) Geometries returned by GetGeometryRef() become stale when its parent
dies.
3)  FieldDefn must have their parent FeatureDefn alive.

I might have these wrong - the whole situation is rather confusing to me.

The only thing which is certainly a problem are bindings for gc
languages.  Even if the developer maintains good habits, the gc might
still collect the trash in the wrong order.  There are a couple of ways
to mitigate this:  We could adopt a "no-fail" destructor strategy.  That
is ensure that under no circumstances will destructors being called in
the wrong order cause troubles.  Quite frankly I don't know how this can
be done.   The other is to have Charlie implement his "backreference in
script-lang" swig hook for Ruby.  His typemap is both ingenous and
devious at the same time and hence has quite a bit of beauty.  My
biggest object to it was it doesn't solve the problem for all
languages.  But if we can sick out our chins and state in large
commanding voices, "There are no problems in python," then we don't have
a problem to solve.

>However, I leave it in your hands.
>
>  
>
I appreciate the confidence.  My preferred solution at this time is to
deny the problem in python and perl and php and lua and require the
developer to follow the C api usage.  And have Charlie mix up his wicked
potion of chicken bones and tea leaves and fix Ruby.

But I do defer to Your Honorable +/-1.

Kevin


_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Frank Warmerdam
On 9/20/05, Kevin Ruland <[hidden email]> wrote:

> The current swig bindings have some short falls but I do believe they
> are fairly useable provided the developer follow the guidelines of the
> underlying C++ library.  The rules in a nutshell are:
>
> 1) you must maintain a handle to the Datasource as long as you are using
> objects from it (with some exceptions such as XXX).  If the Datsource is
> destroyed, developer pays for all damages.
> 2) Geometries returned by GetGeometryRef() become stale when its parent
> dies.
> 3)  FieldDefn must have their parent FeatureDefn alive.
>
> I might have these wrong - the whole situation is rather confusing to me.

Kevin,

I believe your summary is correct.
 
> The only thing which is certainly a problem are bindings for gc
> languages.  Even if the developer maintains good habits, the gc might
> still collect the trash in the wrong order.

I'm not sure why it matters what order things are destroyed in as
long as you hold a reference to things till you are done with them.

>  There are a couple of ways
> to mitigate this:  We could adopt a "no-fail" destructor strategy.  That
> is ensure that under no circumstances will destructors being called in
> the wrong order cause troubles.  Quite frankly I don't know how this can
> be done.  

Perhaps you could clarify where we have problems with gc
languages.  I *feel* this isn't a problem, because language
variables that reference objects should not need to do anything
at all for objects they don't own.  So, for instance, a reference
to a geometry that is owned by some other object does not need
to do any work at all in it's destructor other than check it's
"owned" flag.  

Best regards,
--
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, [hidden email]
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Kevin Ruland

>Perhaps you could clarify where we have problems with gc
>languages.  I *feel* this isn't a problem, because language
>variables that reference objects should not need to do anything
>at all for objects they don't own.  So, for instance, a reference
>to a geometry that is owned by some other object does not need
>to do any work at all in it's destructor other than check it's
>"owned" flag.  
>
>  
>

Frank,


I had had this nice beautiful description of the nasties complete with
alusions to the dispicable state of US politics, but after reading
through the ogr.i code, I wonder if the original source of the problem
is really my bug in the ogr.i interface description

I see that r1.35 of ogr.i on 8/22 I checked in a change which added
%newobject to both Layer.GetFeature and Layer.GetNextFeature.  Recall
that %newobject tells the script that it owns the returned pointer and
will call the destructor (which is OGR_F_Destroy()).

Could this be the source of the original angst?  It probably isn't
because chained calls still wont work because you'll end up with a
dangling reference.  However, it probably does fix my percieved gc
problem between layer and feature.

Unless Charlie can tell me otherwise, I don't think there is a potential
gc problem between Datasource and Layer anymore.  The destructors for
layer are noops so they should be able to succeed after calling the
datasource destructor.  This is of course dependent on the assumption
that the datasource reference is maintained during the lifetime of the
layer.  Charlie has pointed to problems between FeatureDefn and
FieldDefn, but you have already assumed ownership of this.

I think it would be useful for my sanity if you could walk through ogr.i
and look at the various methods which create or consume pointers.  Any
function which returns a pointer owned by the script needs to have
%newobject.  Any function which assumes ownership of a pointer needs to
have a DISOWN typemap.  I see that Layer has things like SetFeature and
CreateFeature, I suspect they should be using DISOWN, but hey what do I
know.

Alas, I thought it was just last week when I apologized for blowing up a
very similar issue over FieldDefn (cf. r1.41).

Kevin
_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Charles F. I. Savage
In reply to this post by Frank Warmerdam
> I'm not sure why it matters what order things are destroyed in as
> long as you hold a reference to things till you are done with them.

Yes, agreed.  As long as you hold the a reference to the parent object
you are fine.

For example (feature owns geometry):

    feature = layer.get_next_feature
    geom = feature.get_geom_refn

    <do stuff with geom>

    // release top level feature
    feature = nil

The one issue could be the chaining issue that's been brought up:

geom = layer.get_next_feature.get_geom_refn

If the dynamic language decides to gc the feature on you during the
expression then you have a problem.  But I would be surprised if any
language actually does this - I would think you are safe within a single
expression.  But I haven't checked.

IMHO I would vote to go down this route instead of implementing lots of
complicated reference counting C++ wrappers.  Both because it will be
really hard to get right, and second, if we start making copies of
everything I worry about performance.

Charlie



>
>>  There are a couple of ways
>> to mitigate this:  We could adopt a "no-fail" destructor strategy.  That
>> is ensure that under no circumstances will destructors being called in
>> the wrong order cause troubles.  Quite frankly I don't know how this can
>> be done.  
>
> Perhaps you could clarify where we have problems with gc
> languages.  I *feel* this isn't a problem, because language
> variables that reference objects should not need to do anything
> at all for objects they don't own.  So, for instance, a reference
> to a geometry that is owned by some other object does not need
> to do any work at all in it's destructor other than check it's
> "owned" flag.  
>
> Best regards,


_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Ari Jolma
In reply to this post by Kevin Ruland
Let me theoretize a bit on this subject, because it confuses me, when
you (seem) to be talking about specific cases. I'd like to see a very
short list of rules, and then think what should happen in each case.

First, in my previous post, forget about Perl be able to detect, but
remember the principles, that we ought to be able to do in-place
manipulation of objects (for speed) but also be able to take an object
from its container and have it although the container goes away.

An object must know about its' container, so that when the object is
really destroyed, the destructor needs to check the container, and
destroy also it, if it isn't referenced by anybody else.

When a container is really destroyed, all the objects that it contains,
are also destroyed. If an object, which is thus being destroyed, is
referenced by somebody, then it must clone itself and continue living.

Am I missing something? Isn't this all about objects, references to
objects and their containers? And the about when an object or container
is really destroyed. There aren't that many possible cases and it
shouldn't be impossible to look at each case and decide what happens.
And, is there a difference between an object and a container? A
container is just an object which owns another object?

If you give an object to a container, it could always be just by
reference. If the original owning variable goes out of scope, the object
reference in the container must clone the real object and become also a
real. We can give it explicitly by value to avoid cloning, though.

I don't see it impossible to implement this in the thin C/C++ layer
between scripts and GDAL. As far as I understand the only important
missing piece is the link from an object to its container. That's also a
rule: an object can be (owned) in only one container.

The variables in the script are always just references to the actual
objects under the hood. The engine below must take care that the correct
objects exist. It is difficult because we're just responding to messages
(like in GUI programming) and do not have complete control.

Ah, maybe this is enough for now ;)

Ari


Kevin Ruland kirjoitti:

>
>> Perhaps you could clarify where we have problems with gc languages.  
>> I *feel* this isn't a problem, because language
>> variables that reference objects should not need to do anything
>> at all for objects they don't own.  So, for instance, a reference
>> to a geometry that is owned by some other object does not need
>> to do any work at all in it's destructor other than check it's
>> "owned" flag.
>>  
>>
>
> Frank,
>
>
> I had had this nice beautiful description of the nasties complete with
> alusions to the dispicable state of US politics, but after reading
> through the ogr.i code, I wonder if the original source of the problem
> is really my bug in the ogr.i interface description
>
> I see that r1.35 of ogr.i on 8/22 I checked in a change which added
> %newobject to both Layer.GetFeature and Layer.GetNextFeature.  Recall
> that %newobject tells the script that it owns the returned pointer and
> will call the destructor (which is OGR_F_Destroy()).
>
> Could this be the source of the original angst?  It probably isn't
> because chained calls still wont work because you'll end up with a
> dangling reference.  However, it probably does fix my percieved gc
> problem between layer and feature.
>
> Unless Charlie can tell me otherwise, I don't think there is a
> potential gc problem between Datasource and Layer anymore.  The
> destructors for layer are noops so they should be able to succeed
> after calling the datasource destructor.  This is of course dependent
> on the assumption that the datasource reference is maintained during
> the lifetime of the layer.  Charlie has pointed to problems between
> FeatureDefn and FieldDefn, but you have already assumed ownership of
> this.
>
> I think it would be useful for my sanity if you could walk through
> ogr.i and look at the various methods which create or consume
> pointers.  Any function which returns a pointer owned by the script
> needs to have %newobject.  Any function which assumes ownership of a
> pointer needs to have a DISOWN typemap.  I see that Layer has things
> like SetFeature and CreateFeature, I suspect they should be using
> DISOWN, but hey what do I know.
>
> Alas, I thought it was just last week when I apologized for blowing up
> a very similar issue over FieldDefn (cf. r1.41).
>
> Kevin
> _______________________________________________
> Gdal-dev mailing list
> [hidden email]
> http://lists.maptools.org/mailman/listinfo/gdal-dev



--
Prof. Ari Jolma
Kartografia ja Geoinformatiikka / Cartography and Geoinformatics
Teknillinen Korkeakoulu / Helsinki University of Technology
POBox 1200, 02015 TKK, Finland
Email: ari.jolma at tkk.fi URL: http://www.tkk.fi/~jolma

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Charles F. I. Savage
In reply to this post by Kevin Ruland
> I see that r1.35 of ogr.i on 8/22 I checked in a change which added
> %newobject to both Layer.GetFeature and Layer.GetNextFeature.  Recall
> that %newobject tells the script that it owns the returned pointer and
> will call the destructor (which is OGR_F_Destroy()).

Yes, this is correct.  These features are new and thus must be owned by
the dynamic language.

> Could this be the source of the original angst?  It probably isn't
> because chained calls still wont work because you'll end up with a
> dangling reference.  However, it probably does fix my percieved gc
> problem between layer and feature.

Not quite sure what you mean - but you could be running into the
FeatureDefn ref count issue.  That causes all sorts of strange errors
when it happens.


> Unless Charlie can tell me otherwise, I don't think there is a potential
> gc problem between Datasource and Layer anymore...This is of course dependent on the assumption
> that the datasource reference is maintained during the lifetime of the
> layer.

Yes, agreed.

> I think it would be useful for my sanity if you could walk through ogr.i
> and look at the various methods which create or consume pointers.  Any
> function which returns a pointer owned by the script needs to have
> %newobject.  Any function which assumes ownership of a pointer needs to
> have a DISOWN typemap.  I see that Layer has things like SetFeature and
> CreateFeature, I suspect they should be using DISOWN, but hey what do I
> know.

I've gone through the OGR api fairly closely (i.e., traced each method
with a debugger) and think what we have now is correct.  By that, I mean
we have %newobject where we should, we have %disown where we should, and
we have identified methods that return child objects that the dynamic
language must let go off before their parents are freed.  However, it
would be a good idea for someone to double check.

As far as setfeature and createfeature, they both clone the features
that they work on.  So no, they should not use disown.

Charlie

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Charles F. I. Savage
In reply to this post by Ari Jolma
> An object must know about its' container, so that when the object is
> really destroyed, the destructor needs to check the container, and
> destroy also it, if it isn't referenced by anybody else.

In OGR an object does not know its container.

> When a container is really destroyed, all the objects that it contains,
> are also destroyed. If an object, which is thus being destroyed, is
> referenced by somebody, then it must clone itself and continue living.

The other way to look at this, and I think a simpler way, is that you
must not use an object once it has been destroyed by its container
(because the container was destroyed).  Thus the discussion centers
around how you keep the container alive.  Is it up to the script writer
(by holding references)?  Or is there something that we can do in the
SWIG interface to help out?   The two possibilities that have been
discussed are reference counting (complex) and having the child
scripting object have a reference to its parent (simple, leverages the
languages garbage collector, but does not work for all languages).

> I don't see it impossible to implement this in the thin C/C++ layer
> between scripts and GDAL. As far as I understand the only important
> missing piece is the link from an object to its container.

There is no such link and thus this would require either an extensive
rewrite of OGR or a whole lot of complicated code in the SWIG binding.
And in the end, I don't see what you gain from it but slower code
because I think there will be edge cases where you can subvert this
layer and get in trouble.  Better to keep things simpler.

 > That's also a rule: an object can be (owned) in only one container.

Already true.

> The variables in the script are always just references to the actual
> objects under the hood. The engine below must take care that the correct
> objects exist. It is difficult because we're just responding to messages
> (like in GUI programming) and do not have complete control.

I actually think the better approach is use the garbage collector of the
scripting language (i.e., how to keep the container alive as discussed
above).  Otherwise we end up with memory managed in 3 places - the
scripting language, the wrapper code, and GDAL/OGR.  Its already hard
enough in 2 places!

Charlie


> Kevin Ruland kirjoitti:
>
>>
>>> Perhaps you could clarify where we have problems with gc languages.  
>>> I *feel* this isn't a problem, because language
>>> variables that reference objects should not need to do anything
>>> at all for objects they don't own.  So, for instance, a reference
>>> to a geometry that is owned by some other object does not need
>>> to do any work at all in it's destructor other than check it's
>>> "owned" flag.  
>>>
>>
>> Frank,
>>
>>
>> I had had this nice beautiful description of the nasties complete with
>> alusions to the dispicable state of US politics, but after reading
>> through the ogr.i code, I wonder if the original source of the problem
>> is really my bug in the ogr.i interface description
>>
>> I see that r1.35 of ogr.i on 8/22 I checked in a change which added
>> %newobject to both Layer.GetFeature and Layer.GetNextFeature.  Recall
>> that %newobject tells the script that it owns the returned pointer and
>> will call the destructor (which is OGR_F_Destroy()).
>>
>> Could this be the source of the original angst?  It probably isn't
>> because chained calls still wont work because you'll end up with a
>> dangling reference.  However, it probably does fix my percieved gc
>> problem between layer and feature.
>>
>> Unless Charlie can tell me otherwise, I don't think there is a
>> potential gc problem between Datasource and Layer anymore.  The
>> destructors for layer are noops so they should be able to succeed
>> after calling the datasource destructor.  This is of course dependent
>> on the assumption that the datasource reference is maintained during
>> the lifetime of the layer.  Charlie has pointed to problems between
>> FeatureDefn and FieldDefn, but you have already assumed ownership of
>> this.
>>
>> I think it would be useful for my sanity if you could walk through
>> ogr.i and look at the various methods which create or consume
>> pointers.  Any function which returns a pointer owned by the script
>> needs to have %newobject.  Any function which assumes ownership of a
>> pointer needs to have a DISOWN typemap.  I see that Layer has things
>> like SetFeature and CreateFeature, I suspect they should be using
>> DISOWN, but hey what do I know.
>>
>> Alas, I thought it was just last week when I apologized for blowing up
>> a very similar issue over FieldDefn (cf. r1.41).
>>
>> Kevin
>> _______________________________________________
>> Gdal-dev mailing list
>> [hidden email]
>> http://lists.maptools.org/mailman/listinfo/gdal-dev
>
>
>


_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Ari Jolma
Charlie Savage wrote:

>> An object must know about its' container, so that when the object is
>> really destroyed, the destructor needs to check the container, and
>> destroy also it, if it isn't referenced by anybody else.
>
>
> In OGR an object does not know its container.


Yes, and that I see as the missing piece. But I'm willing to live
without it.

> I actually think the better approach is use the garbage collector of
> the scripting language (i.e., how to keep the container alive as
> discussed above).


Those languages which rely on reference counting in gc, can't handle a
case where a container, which is kept alive by increasing its reference
count, is not referenced by any variable -- it just does not get deleted
at all, thus in

lyr = datasource("xx").getlayer

the anonymous datasource object does not get deleted if lyr is a working
reference to its part.

Either lyr has to become a stale variable so that it is noticeable in
the variable (and its subsequent use can be blocked automatically), or
something in lyr has to know that the anonymous datasource object is its
parent, i.e., have a pointer to it (see above).

Ari


--
Prof. Ari Jolma
Kartografia ja Geoinformatiikka / Cartography and Geoinformatics
Teknillinen Korkeakoulu / Helsinki University of Technology
POBox 1200, 02015 TKK, Finland
Email: ari.jolma at tkk.fi URL: http://www.tkk.fi/~jolma

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: GetGeometryRef in scripts

Frank Warmerdam
In reply to this post by Ari Jolma
On 9/21/05, Ari Jolma <[hidden email]> wrote:
> Am I missing something? Isn't this all about objects, references to
> objects and their containers? And the about when an object or container
> is really destroyed. There aren't that many possible cases and it
> shouldn't be impossible to look at each case and decide what happens.
> And, is there a difference between an object and a container? A
> container is just an object which owns another object?

Ari,

You express some desirable rules (such as if you hold a reference
to a member of a container and the container is destroyed, the member
should be copied to keep the reference valid) but I just feel that these
rules are not practical to support because the underlying OGR
objects provide no help.

For instance, OGRFeatureDefn, OGRSpatialReference and
OGRDataSource are all referenced counted in C++.  So we can
ensure that any direct scripting variable referencing them takes
a reference.  However OGRLayer, and OGRGeometry are not
reference counted, and it is very difficult to give them full
object semantics as you wish.

Charlie says:
> There is no such link and thus this would require either an extensive
> rewrite of OGR or a whole lot of complicated code in the SWIG binding.
> And in the end, I don't see what you gain from it but slower code
> because I think there will be edge cases where you can subvert this
> layer and get in trouble.  Better to keep things simpler.

I agree with this.  Attempt to build huge amounts of reference
holding logic into the swig interface is likely to make things so
complicated that I am unable to touch the swig stuff anymore.
Better to keep things simple and understandable.

I realize this leaves scripters in the position of having to follow
some conventions and risk crashes if they don't.  This is
unfortunate and indicates an imperfect packaging but it is really
because the underlying library was not designed for the kind of
use you wish.

Best regards,
--
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, [hidden email]
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev