GNOME Bugzilla – Bug 346946
Load types dynamically
Last modified: 2010-12-23 21:01:49 UTC
PyGTK does currently create wrappers for all known classes, interfaces, enums, flags on startup instead of when they are actually used the first time. This takes quite some time and uses up quite a bit of memory for things that are not used in most applications.
The gobject part of this problem is reported in bug 346947
Created attachment 68596 [details] [review] v1: work in progress I kind of got stuck in here, I can't really figure out how to make a module dynamic (and also keep gtk._gtk/gtk.glade imports still working) OR how to make a type dynamic and then replace it when it's actually used. I added some simple tests to verify that gobject.new() actually works.
From irc discussions with jdahlin, it looks like libglade and probably other modules use something like the following: if ((module = PyImport_ImportModule("gtk._gtk")) != NULL) { PyObject *moddict = PyModule_GetDict(module); _PyGtkWidget_Type = (PyTypeObject *)PyDict_GetItemString(moddict, "Widget"); This means that gtk._gtk must be a module object (PyModuleObject instance) and that it's dictionary must contain a value that is a PyTypeObject instance. I don't think lazy creation of type instances is going to work because of this. Even if dict was replaced with a subclass with __missing__ defined (a 2.5? feature) PyDict_GetItemString would not call it. If the code was something along the lines of: if ((module = PyImport_ImportModule("gtk._gtk")) != NULL) { _PyGtkWidget_Type = (PyTypeObject *)PyObject_GetAttrString(module, "Widget"); then a variety of lazy techniques probably would be possible. If might still be possible to lazily create method objects on the type objects. I don't know how big of a win this would be or how difficult it is (though probably not too difficult).
Comment on attachment 68596 [details] [review] v1: work in progress >+ def _write_get_symbol_names(self, writer): >+ self.fp.write("""static PyObject * >+_wrap__get_symbol_names(PyObject *self) [...] >+_wrap__get_symbol(PyObject *self, PyObject *args) [...] Is this to support lazy loaded methods? if so, it should be clear in the bug title then.. >+_wrap__get_symbol(PyObject *self, PyObject *args) >+{ >+ PyObject *d; >+ char *name; >+ if (!PyArg_ParseTuple(args, "Os", &d, &name)) >+ return NULL; >+""") >+ first = True >+ for obj, bases in writer.get_classes(): >+ if first: >+ self.fp.write(' if (!strcmp(name, "%s")) {\n' % obj.name) >+ first = False >+ else: >+ self.fp.write(' } else if (!strcmp(name, "%s")) {\n' % obj.name) >+ self.fp.write( >+ ' return (PyObject*)pygobject_lookup_class(%s);\n' % >+ obj.typecode) >+ self.fp.write(' }\n') >+ self.fp.write(' return Py_None;\n}\n\n'); Ugh!!! This is awful coding; a series of strcmp's is going to be slow! Remember, we are trying to increase startup performance, not make pygtk programs crawl. I really think we should move one step at a time, lazy loaded types should be more than enough for now. >+%s_register_type(GType type, PyObject *unused) >+{ >+ PyObject *m = PyImport_ImportModule("gtk"); >+ PyObject *d = PyModule_GetDict(m); >+""" % obj.c_name) >+ writer.write_class(obj, bases, indent=1) [without looking at the generated code] So, we have here a foo_bar_register_type() function, and inside we have PyTypeObject PyFooBar_Type At least I don't see a 'static' being added anywhere, so I assume it will have no 'static', and so the PyTypeObject is stack allocated, which is obviously wrong; it will be deallocated when the function exists. Considering my assumptions are correct, you are going to change that to 'static', and then I ask, why do we need to generate one xxxx_register_type for each type? Couldn't we have a more simple API: pyg_register_lazy_type(const gchar *typename, PyTypeObject *pytype, PyCFunction type_get_symbol_names, PyCFunction type_get_symbol); Thereby saving <NTYPES> generated functions. >+ self.fp.write("}\n") >+ >+ functions.append(' { "_get_symbol_names", ' >+ '(PyCFunction)_wrap__get_symbol_names, ' >+ 'METH_NOARGS, NULL },\n') >+ functions.append(' { "_get_symbol", ' >+ '(PyCFunction)_wrap__get_symbol, ' >+ 'METH_VARARGS, NULL },\n') >+ > # write the PyMethodDef structure > functions.append(' { NULL, NULL, 0, NULL }\n') > >+class LazyClass: #(object): >+ def __init__(self, mod, name): >+ self._mod = mod >+ self._name = name >+ >+ import sys >+ f = sys._getframe(2) >+ f.f_globals[self._name] = self >+ self._locals = f.f_globals >+ >+ def __getattr__(self, name): >+ #print name, self._locals, self._name >+ obj = self._mod._get_symbol(self._locals, self._name) >+ self._locals[self._name] = obj >+ return getattr(obj, name) Ugh!... I hate sys._getframe() :| Again, do we really need this now?
> >+class LazyClass: #(object): > >+ def __init__(self, mod, name): > >+ self._mod = mod > >+ self._name = name > >+ > >+ import sys > >+ f = sys._getframe(2) > >+ f.f_globals[self._name] = self > >+ self._locals = f.f_globals > >+ > >+ def __getattr__(self, name): > >+ #print name, self._locals, self._name > >+ obj = self._mod._get_symbol(self._locals, self._name) > >+ self._locals[self._name] = obj > >+ return getattr(obj, name) > > Ugh!... I hate sys._getframe() :| Again, do we really need this now? > WIP, that part is unsolved. I still don't know how to solve the Lazy loading of types. Tried a lazynamespace and lazyclass, both of them have serious problems. Overriding/modifying sys.modules is the most attractive option, but I have not yet figured out how to make from gtk.glade import properly. That's what I the most immediate problem to fix.
(In reply to comment #4) > >+_wrap__get_symbol(PyObject *self, PyObject *args) > >+{ > >+ PyObject *d; > >+ char *name; > >+ if (!PyArg_ParseTuple(args, "Os", &d, &name)) > >+ return NULL; > >+""") > >+ first = True > >+ for obj, bases in writer.get_classes(): > >+ if first: > >+ self.fp.write(' if (!strcmp(name, "%s")) {\n' % obj.name) > >+ first = False > >+ else: > >+ self.fp.write(' } else if (!strcmp(name, "%s")) {\n' % obj.name) > >+ self.fp.write( > >+ ' return (PyObject*)pygobject_lookup_class(%s);\n' % > >+ obj.typecode) > >+ self.fp.write(' }\n') > >+ self.fp.write(' return Py_None;\n}\n\n'); > > Ugh!!! This is awful coding; a series of strcmp's is going to be slow! > Remember, we are trying to increase startup performance, not make pygtk > programs crawl. I really think we should move one step at a time, lazy loaded > types should be more than enough for now. Sorry, I read this wrong the first time; this code is attached to module objects lookup, not to type objects. So the penalty is really negligible (assuming after the first lookup the type gets cached in the module dictionary)...
Fixed in CVS. Checking in ChangeLog; /cvs/gnome/gnome-python/pygtk/ChangeLog,v <-- ChangeLog new revision: 1.1539; previous revision: 1.1538 done Checking in configure.in; /cvs/gnome/gnome-python/pygtk/configure.in,v <-- configure.in new revision: 1.160; previous revision: 1.159 done Checking in codegen/codegen.py; /cvs/gnome/gnome-python/pygtk/codegen/codegen.py,v <-- codegen.py new revision: 1.115; previous revision: 1.114 done Checking in codegen/defsparser.py; /cvs/gnome/gnome-python/pygtk/codegen/defsparser.py,v <-- defsparser.py new revision: 1.22; previous revision: 1.21 done Checking in gtk/Makefile.am; /cvs/gnome/gnome-python/pygtk/gtk/Makefile.am,v <-- Makefile.am new revision: 1.72; previous revision: 1.71 done Checking in gtk/__init__.py; /cvs/gnome/gnome-python/pygtk/gtk/__init__.py,v <-- __init__.py new revision: 1.44; previous revision: 1.43 done RCS file: /cvs/gnome/gnome-python/pygtk/gtk/_gtk.py,v done Checking in gtk/_gtk.py; /cvs/gnome/gnome-python/pygtk/gtk/_gtk.py,v <-- _gtk.py initial revision: 1.1 done Checking in gtk/_lazyutils.py; /cvs/gnome/gnome-python/pygtk/gtk/_lazyutils.py,v <-- _lazyutils.py new revision: 1.2; previous revision: 1.1 done Checking in gtk/deprecation.py; /cvs/gnome/gnome-python/pygtk/gtk/deprecation.py,v <-- deprecation.py new revision: 1.2; previous revision: 1.1 done Checking in gtk/gtk.override; /cvs/gnome/gnome-python/pygtk/gtk/gtk.override,v <-- gtk.override new revision: 1.395; previous revision: 1.394 done Checking in gtk/gtkmodule.c; /cvs/gnome/gnome-python/pygtk/gtk/gtkmodule.c,v <-- gtkmodule.c new revision: 1.69; previous revision: 1.68 done Checking in gtk/libglade.override; /cvs/gnome/gnome-python/pygtk/gtk/libglade.override,v <-- libglade.override new revision: 1.30; previous revision: 1.29 done Checking in gtk/libglademodule.c; /cvs/gnome/gnome-python/pygtk/gtk/libglademodule.c,v <-- libglademodule.c new revision: 1.9; previous revision: 1.8 done Checking in gtk/pygtk.h; /cvs/gnome/gnome-python/pygtk/gtk/pygtk.h,v <-- pygtk.h new revision: 1.30; previous revision: 1.29 done Checking in tests/common.py; /cvs/gnome/gnome-python/pygtk/tests/common.py,v <-- common.py new revision: 1.16; previous revision: 1.15 done Checking in tests/test_api.py; /cvs/gnome/gnome-python/pygtk/tests/test_api.py,v <-- test_api.py new revision: 1.2; previous revision: 1.1 done
(In reply to comment #6) > > Ugh!!! This is awful coding; a series of strcmp's is going to be slow! > > Remember, we are trying to increase startup performance, not make pygtk > > programs crawl. I really think we should move one step at a time, lazy loaded > > types should be more than enough for now. > > Sorry, I read this wrong the first time; this code is attached to module > objects lookup, not to type objects. So the penalty is really negligible > (assuming after the first lookup the type gets cached in the module > dictionary)... > After looking at the generated code for gtk.c, I find the generated function _wrap__get_symbol simply horrendous: it contains nearly on thousand chained strcmp else if's. I can't believe any ammount of optimisation gained by lazy loading can compensate for this slowness. Sure, you might gain a bit of memory, but at a tremendous cost in terms of CPU. Also, after cleaning up pygtk to remove the old _gtk.so, pygtk is starting to crash when gnome-python tries to access gtk._gtk to obtain _PyGtk_API (before, gnome-python was importing it from the old _gtk.so which takes precedence over _gtk.py).
> After looking at the generated code for gtk.c, I find the generated function > _wrap__get_symbol simply horrendous: it contains nearly on thousand chained > strcmp else if's. I can't believe any ammount of optimisation gained by lazy > loading can compensate for this slowness. Sure, you might gain a bit of > memory, but at a tremendous cost in terms of CPU. This can easily be optimized if needed, by using a switch for the first character in the string or even a hash table with name -> creation func. I'd be happy too do that if profiling shows that it's an issue. Remember it's only done once per type. > Also, after cleaning up pygtk to remove the old _gtk.so, pygtk is starting to > crash when gnome-python tries to access gtk._gtk to obtain _PyGtk_API (before, > gnome-python was importing it from the old _gtk.so which takes precedence over > _gtk.py). I'd prefer not adding logic checking so _gtk is not imported using the old binary. It should only happen when mixing an old release built from CVS or tarballs with a new one, but perhaps it's enough justification.
(In reply to comment #9) > Remember it's only done once per type. Not just types, also for all global functions and all constants/enums/flags values.
Created attachment 69421 [details] [review] enable dynamic namespace for gtk Okay, I reverted this because of the PyModule_GetDict. It's AFAIK impossible to get this working: import gtk gtk.__dict__['Dialog'] Which is basically what all old extension modules are doing. The code generator changes are still there, but not used for anything.
Hmm, that's a problem and I do think it's impossible to get gtk.__dict__['Dialog'] working. Options I see are: 1) Break compatibility 2) Abandon lazy loading of top level 3) Create another module such as perhaps pygtk2.gtk to do the lazy loading and migrate apps to use it by changing import gtk to from pygtk2 import gtk 4) Create a mechanism for an application to say it wants to use lazy loading, which will probably work for something like a self-contained applet but not for an extensible application like Gazpacho.
The comment that should have accompanied marking this bug RESOLVED: Lazy loading has been implemented in PyGObject with GObject Introspection. The gobject related bug 346947 Johan mentions in comment 1 has already been closed as resolved/obsolete.