Hi, Thomas:
Thanks for the help. I changed TopoDS_Shape method to this:
def __del__(self):
self.thisown=False;
self._kill_pointed();
self.thisown=False was required to suppress swig warnings of memory leak,
cannot find destructor
This helped performance a lot. Profiled execution time for my snipped below
went from 11ms of cpu time to this:
ncalls tottime percall cumtime percall filename:lineno(function)
1 0.000 0.000 0.006 0.006 <string>:1(<module>)
1 0.002 0.002 0.006 0.006
curveIntersections.py:68(loopWire2)
155 0.001 0.000 0.002
0.000c:\python266\lib\site-packages\OCC\TopoDS.py:928(__del__)
155 0.001 0.000 0.001 0.000
c:\python266\lib\site-packages\OCC\TopoDS.py:741(<lambda>)
155 0.000 0.000 0.000 0.000
{_TopTools.TopTools_HSequenceOfShape_Append}
this is approximately 50% faster than before. When i run the test 'for real'
outside the profiler, the speedup is more, oddly: about 3x faster.
I will leave this modification in for a while to see if it works ok. I would
say based on this test that your new approach is much better.
thanks again for the help!
On Thu, Mar 3, 2011 at 3:30 AM, Thomas Paviot <tpav...@gmail.com> wrote:
> 2011/3/3 Dave Cowden <dave.cow...@gmail.com>
>
>> Hi, everyone:
>>
>> I'm performance tuning some code, and running into an issue that I could
>> use some help with. When I run this code:
>>
>> def loopWire2(w):
>> edges = TopTools.TopTools_HSequenceOfShape();
>> topexp = TopExp.TopExp_Explorer();
>> topexp.Init(w,TopAbs.TopAbs_EDGE);
>> while topexp.More():
>> edges.Append(topexp.Current());
>> topexp.Next();
>> return edges;
>>
>> s = loopWire2( ( a wire with about 155 linear edges ) );
>>
>> ..and I profile it as it runs with cProfile, i see something like this:
>>
>> ncalls tottime percall cumtime percall filename:lineno(function)
>> 1 0.000 0.000 * 0.009 * 0.009 <string>:1(<module>)
>> 1 0.002 0.002 0.009 0.009
>> curveIntersections.py:67(loopWire2)
>> 155 0.001 0.000 *0.005* 0.000
>> c:\python266\lib\site-packages\OCC\TopoDS.py:928(__del__)
>> 157 0.002 0.000 *0.003* 0.000
>> c:\python266\lib\site-packages\OCC\GarbageCollector.py:75(collect_object)
>> 155 0.001 0.000 0.001 0.000
>> c:\python266\lib\site-packages\OCC\TopoDS.py:741(<lambda>)
>>
>> Note that a large portion of the execution time is used in creating and
>> collecting TopoDS objects, which i assume are implicitly created in
>> TopExp_Explorer.
>>
>> I first noticed this when code that was iterating over wires with large
>> edge counts was taking a longer than usual time. Though the above runs
>> pretty fast ( 4ms in real time, 9ms profiled ), it doesnt take long for this
>> to add up fast. The code above is an optimized version of what i started
>> with, which ran twice as slow as this one because I used casts to
>> topds_edge, which created more objects that had to be collected.
>>
>> I'm sure I could disable the garbage collector, but that would just defer
>> the problem to later.
>>
>> Does anyone know how to prevent the extra overhead in this case? I have
>> also tried WireExplorer and Utils.Topology-- they appear to perform worse--
>> though mostly due to the same issue: collecting garbage.
>>
>> So, I am at a loss ( besides going to C++ code--ew ) on how to make the
>> above code run faster. Looping through 155 edges shouldnt take 4ms should
>> it?
>>
>> Here are my operating parameters:
>>
>> windows xp (32 bit )
>> python 2.6.6 (32 bit )
>> pythonocc 0.5 ( installed with all-in-one: thus OCC 6.3.0 )
>>
>> thanks in advance for the ideas/help!
>>
>>
> Hi Dave,
>
> Interesting issue, it's been a long time we didn't discuss the GC here.
>
> A brief recall about the GarbageCollector first: the very early version of
> pythonOCC didn't implement any GC. Everything seemed ok except a few
> function calls with strange behavior and program crashes. pythonOCC 0.2 was
> able to run the following code:
> box = BRepPrimAPI_MakeBox(10,10,10)
> box_shape = box.Shape()
> assert (box_shape.IsNull() is False)
>
> However, the following was raising an AssertException :
> box_shape = BRepPrimAPI_MakeBox(10,10,10).Shape()
> assert (box_shape.IsNull() is False)
>
> The reason is that the Shape() method of the BRepPrimAPI_MakeBox returns a
> *reference* to the TopoDS_Shape. In the second example, the
> BRepPrimAPI_MakeBox(10,10,10) instance is deleted after the first line, the
> box_shape then points to a null shape. Many other segfaults or program
> crashes had the same cause. I then had to find a way to fix that, and I
> decided to prevent object deletion by overloading the __del__ method of each
> object, move the object to a garbage collector, and wait for an explicit
> statement to kill it.
>
> For each pythonOCC available class, you can see that the following code was
> added:
> def __del__(self):
> try:
> self.thisown = False
> GarbageCollector.garbage.collect_object(self)
> except:
> pass
>
> This part of code just moves the object to the GarbageCollector. The
> 'self.thisown=False' tells the SWIG wrapper that the pointer to the C/C++
> instance should not be deleted. Then the object is collected by the
> collect_object() method:
> def collect_object(self, obj_deleted):
> ''' This method is called whenever a pythonOCC instance is
> deleted.'''
> #self._collected_objects.append(obj_deleted)
> if hasattr(obj_deleted,'was_purged'):
> return False
> if obj_deleted.__class__.__name__.startswith('Handle'):
> self._handles.append(obj_deleted)
> logger.info('collected Handle')
> elif hasattr(obj_deleted,"GetHandle"):
> self._transients.append(obj_deleted)
> logger.info('collected transient')
> else:
> self._misc.append(obj_deleted)
> logger.info('collected misc')
>
> The 'dying' object is just copied to a list according to its type. The
> instance is killed when the _kill_pointed() method is called. This method is
> defined at the SWIG/C++ level:
> void _kill_pointed() {
> delete $self;
> }
>
> You can notice that, for each object deletion, *a lot* of code is ran by
> the __del__/collect() methods. In your code, after the line
> edges.Append(topexp.Current()), the topexp.Current() instance is being
> deleted and then moved to the GarbageCollector. The previous part of code is
> then ran 155 times, leading to a long time (4ms) compared to the low number
> of instances you're working with.
>
> This GarbageCollector is obviously an inelegant way to deal with the
> intensive use of &references by the OCC C++ kernel. I've been experiencing a
> better way to overcome this issue: each method that returns, in C++, a
> reference to an object should return, in Python, a copy of this object. This
> feature is already implemented in the 0.5 release for the classes of the gp*
> and TopoDS* modules. In pythonOCC-0.5, the Current() method of the TopExp()
> classe uses the copy constructor of the TopoDS_Shape class to create this
> copy and return the object. In order to understand this difference, you can
> have a look at the SWIG code:
> pythonOCC-0.4, file TopExp.i:
>
> const TopoDS_Shape & Current() const;
>
>
> pythonOCC-0.5, file TopExp.i:
>
> const TopoDS_Shape Current() const;
>
>
> There is a very small difference : the '&' disappeared from the wrapper,
> but it changes everything: the returned shape is not a reference anymore.
> The topexp.Current() instance can be safely deleted, and the GC is not
> required anymore. However, this is still an experimental development, and I
> did not disabled the GC for the gp* and TopoDS* objects, since other methods
> should return reference to these objects and I'd like to be sure that this
> approach does not introduce any regression.
>
> So now I come to the answer to your question. If you want to speed up the
> code, two solutions can be explored:
> 1. Imagine a code where the __del__ method of the topexp.Current() is never
> called. Don't know whether or not it is possible
> 2. edit/modify the TopoDS.py file so that the __del__ method of the
> TopoDS_Shape, TopoDS_Face etc. calls the _kill_pointed() method (this should
> be the default behavior). This should speed up your of code by a factor 10
> to 100.
> def __del__(self):
> self._kill_pointed()
>
> In the future, I plan to wrap the default destructor of the object rather
> than adding a "_kill_pointed" method.
>
> Hope this helps,
>
> Thomas
>
>
>
>
>
>
> _______________________________________________
> Pythonocc-users mailing list
> Pythonocc-users@gna.org
> https://mail.gna.org/listinfo/pythonocc-users
>
>
_______________________________________________
Pythonocc-users mailing list
Pythonocc-users@gna.org
https://mail.gna.org/listinfo/pythonocc-users
