[Kde-bindings] Problem condition in Qyoto MethodCall

[Richard Dale]

On Monday 12 February 2007, Arno Rehn wrote:
> Am Montag, 12. Februar 2007 schrieb Richard Dale:
> > On Monday 12 February 2007, Arno Rehn wrote:
> > > > [ ... cut ... ]
> > >
> > > Well, I've hacked a lot around in qyoto.cpp and changed the way we
> > > check if it's a contained instance or not: I've added a field to
> > > smokeqyoto_object called 'contained' and it is set up when the
> > > constructor was just called. This was necessary because sometimes the
> > > instance was already destroyed and we still wanted to get the parent of
> > > it. The new field is updated when the parent of the widget is changed
> > > because it is added to a layout, too. It works better now and up to t10
> > > every tutorial works well. But with t11 it keeps crashing and I can't
> > > figure out why it does so. Since I've created quite a mess, I don't
> > > want to check in what I have. I've attached a patch with all the
> > > changes, if you could take a look at it and tell me what you think...
> >
> > I'm not sure if this is the best way to do it, as 'contained' seems to
> > duplicate the function of 'allocated'. If an instance is 'owned' by
> > another instance we can just set 'allocated' to false, and the Qyoto
> > runtime won't delete it when the corresponding C# instance is garbage
> > collected.
>
> Yes, didn't think about that, but it's right. Setting allocated to false
> would be better.
> Do I see that right: If allocated has to be true for a destructor to be
> called, we only want to call destructors for instances which have been
> created by a C# constructor? Then we'd just have to check if it's a
> destructor and if getPointerObject returns something other than 0.
>
> > What problem does it solve that the current scheme with the
> > IsContainedInstance() function doesn't do?
>
> The problem is the following: We have a QLabel which is a child to a
> QWidget, for example. Now the QWidget is destroyed and with it the QLabel
> is gone. Nevertheless the Qyoto runtime wants to call the destructor for
> the QLabel as it still 'thinks' the QLabel exists. To check whether QLabel
> is a child to something and whether to call the destructor we call
> IsContainedInstance() which checks for any parents. Thus it will cause an
> error when trying to call QLabel->parent() as the QLabel is already
> destroyed. Therefore it's better to set this property up when the instance
> is created and when the parent is changed.
All destructors in the Smoke library have a callback to 
QyotoSmokeBinding::deleted(), and 'o->ptr' for the instance is set to 0 
there:

	void deleted(Smoke::Index classId, void *ptr) {
		void * obj = getPointerObject(ptr);
		smokeqyoto_object *o = value_obj_info(obj);
	
		if(do_debug & qtdb_gc) {
			qWarning("%p->~%s()", ptr, smoke->className(classId));
		}
	
		if(!o || !o->ptr) {
			return;
		}
		unmapPointer(o, o->classId, 0);
		o->ptr = 0;
		
		// delete the previously created QSignalSpy
		if (strcmp("QApplication", smoke->className(classId)) == 0
			|| strcmp("QCoreApplication", smoke->className(classId)) == 0) {
			
			delete qapp_spy;
			qapp_spy = 0;
		}
    }

I'm not clear why we need to signal spy. Can't you just connect a slot to the 
QApplication aboutToQuit() signal and set a global flag in there?

> > I don't think there's any hurry to solve the problem straight away, and
> > it's best to keep experimenting like you're doing, but not actually
> > commiting anything.
>
> Yes, that's what I thought, too.
>
> > If we use the 'Transfer' data in the PyQt sip files we would need to
> > change ownership when a method, or argument within a method was marked
> > with 'Transfer'. So that data could either go into the smoke library
> > runtime, as it would be useful for every language. Another way would be
> > to add it as C# Attributes, in the 'SmokeMethod' one perhaps.
>
> What exactly is this 'Transfer' data used in PyQt? Haven't heard of it
> before...
Have a look at Phil Thompson's mails from a few days ago. I just gives us some 
metadata about which methods change ownership of an instance. Otherwise, we 
would have to go though every method in Qt and decide whether or not in gave 
or removed an owner for an instance.

> > Qyoto seems a bit slow to me - I expected it to be faster than QtRuby,
> > but it's actually slower at the moment. I tried running mono with the
> > '--profile' option and there doesn't seem to be any obvious method that
> > is hogging the time. The methodId caching scheme where the methodId was
> > put inside the SmokeMethod didn't work, and so there seems to be
> > something strange about how Attributes work. Now I've added caching of
> > methodIds again, it didn't seem to make much difference to the speed.
>
> I think all the marshalling functions make Qyoto so slow, because for a
> simple marshalling a whole bunch of C# methods is called from C++ and vice
> versa. But I only recognize a slowdown when I start an application up. The
> mono runtime itself needs some time to start and then then Qyoto runtime
> needs some little time. But when it finally runs it's quite fast from what
> I've seen so far.
Yes, that might be true. The marshalling a strings <-> QStrings could 
certainly be speeded up a think, as it makes more round trips between C++ and 
C# than it needs to. I haven't tried profiling the C/C++ side yet, maybe that 
might give some more info. It might be a matter of writing test programs to 
find out what is slow. I think it's 'good enough' for now anyway, and it's 
more important to be feature complete and correct than fast at the moment.

-- Richard