GNOME Bugzilla – Bug 704037
Replace gi function wrapping with direct static info objects
Last modified: 2013-07-27 04:59:33 UTC
This is a performance idea which will cut the number of function objects in half, and might also optimize call times because we remove an additional indirection of python calls. We use a decorator for wrapping every function/method brought in by gi. By providing the double-under attributes used for function objects directly on the info struct, we would no longer need the wrapped function objects. We essentially do the following in types.py: Foo.bar = Function(bar_info) https://git.gnome.org/browse/pygobject/tree/gi/types.py?id=3.8.3#n150 We could remove this wrapping by providing __call__ and the other double-under attribute directly on the info objects, cutting the code down to: Foo.bar = bar_info The following attributes/aliases would need to be added to various info structs for this to work: __name__ __module__ __doc__ __call__ https://git.gnome.org/browse/pygobject/tree/gi/pygi-info.c?id=3.8.3#n657 __doc__ should become a lazily evaluated attribute which calls into a registered doc string generator written in Python. This requires a new Python gi module API to set the doc string generator, something along the lines of: import gi def func(giinfo): return "" gi.register_doc_string_generator(func)
Taking a look at this.
Created attachment 249045 [details] [review] Move doc string generator into separate module Move the doc string generator for creating function signatures into "gi.docstring". This includes a new API for getting and setting the doc string creation functions: gi.docstring.get_doc_string_generator gi.docstring.set_doc_string_generator gi.docstring.generate_doc_string Beyond adding the ability for custom doc string generators, this API is a necessary step for adding lazy __doc__ attribute access for optimization.
Created attachment 249097 [details] [review] Remove redundant info.get_name calls Remove a number of redundant calls to GIBaseInfo.get_name. Info names are already cached on function objects so re-use them when possible. This gives a small load time improvement by removing over 2000 calls when importing Gtk. $ cat profile_gtk.py from gi.repository import Gtk $ python -m cProfile -s tottime profile_gtk.py ncalls tottime percall cumtime percall filename:lineno(function) 8541 0.004 0.000 0.004 0.000 {method 'get_name' of 'gi.BaseInfo' objects} After patch: 6506 0.003 0.000 0.003 0.000 {method 'get_name' of 'gi.BaseInfo' objects}
Created attachment 249100 [details] Add API for explicitly enabling doc strings (gi.enable_doc_strings) Replace default of always generating doc strings with requiring explicit enablement. While doc strings are nice to have by default, the majority of run times will not make use of them. We gain roughly a %15 import time performance improvement by not generating them. Use: gi.enable_doc_strings() enable doc strings. $ cat profile_gtk.py from gi.repository import Gtk $ cat ntime TIMEFORMAT='scale=3; %3U + %3S' total=0.0 count=$1 shift for i in $(seq 1 $count) do current=$(echo $( { time $@ 2>/dev/null; } 2>&1 ) | bc) total=$(echo "scale=3; $total + $current" | bc) done echo "scale=3; $total / $count" | bc $ ntime 100 python profile_gtk.py .085 After patch: $ ntime 100 python profile_gtk.py .072
Review of attachment 249100 [details]: Needs test updates.
Created attachment 249102 [details] [review] Merge method and constructor setup Merge _setup_constructors into _setup_methods as they contain same basic logic. This removes an unnecessary call with additional filtering of GIObjectInfo.get_methods() which can be large for objects with many methods. This is a small change which may give a %1 or %2 percent speedup. Before: $ ntime 100 python profile_gtk.py .072 After: $ ntime 100 python profile_gtk.py .070
Created attachment 249103 [details] [review] Merge method and constructor setup Fixed incorrect constructor wrapping.
Created attachment 249320 [details] [review] Add common attribute accessors to PyGIBaseInfo Add __name__, __module__, and __doc__ accessors to PyGIBaseInfo object. This is a precursory patch for setting up PyGICallableInfo as a directly callable object with lazy doc string evaluation.
Review of attachment 249100 [details]: No longer necessary as lazy evaluation will work out.
Created attachment 249335 [details] [review] Add callable and descriptor protocols to PyGICallableInfo Add tp_call (__call__) function to callable info objects. This allows for replacement of wrapped invoke methods directly with the already created callable info object. This has the additional side affect of making doc strings lazily bound (only generated when __doc__ is accessed). Add tp_desc_get which returns a bound version of the callable info for both methods and constructors. Update various internal type checks to reflect the changes. Update tests to reflect the new callable type being the same accross Python 2 & 3. This patch gives roughly a %17 speedup for Gtk imports and an %11 speedup for single argument GI method calls. Additional notes on performance numbers: - Tests were run using Python 2.7 with debug builds of the GNOME stack - Import times were tested using the "ntime" script from comment #4 - Method call times were tested using the %timeit in ipython. Before patch: $ ntime 100 python profile_gtk.py .088 $ ipython In [1]: from gi.repository import Gtk In [2]: button = Gtk.Button() In [3]: %timeit button.get_size_request() 100000 loops, best of 3: 3.43 us per loop After: $ ntime 100 python profile_gtk.py .073 $ ipython ... In [3]: %timeit button.get_size_request() 100000 loops, best of 3: 3.04 us per loop
Created attachment 249343 [details] [review] Fixed spelling mistakes in commit message.
Review of attachment 249343 [details] [review]: ::: gi/pygi-info.c @@ +403,3 @@ + */ +static PyObject * +_callable_info_call (PyGICallableInfo *self, PyObject *args, PyObject *kwargs) This needs to be broken out down into separate functions. A generic version for GICallableInfo and a specialized version for GIFunctionInfo (and eventually GIVFuncInfo) @@ +482,3 @@ + */ +static PyObject * + "%s constructor cannot be used to create instances of " Same comment as above
Created attachment 249440 [details] [review] Add callable and descriptor protocols to PyGICallableInfo Add tp_call (__call__) function to callable info objects. This allows for replacement of wrapped invoke methods directly with the already created callable info object. This has the additional side effect of making doc strings lazily bound (only generated when __doc__ is accessed). Add tp_desc_get (__get__) to PyGIFunctionInfo which returns a bound version of itself for methods and constructors. Update various internal type checks to reflect the changes. Update tests to reflect the new callable type being the same across Python 2 & 3. This patch gives roughly a %17 speedup for Gtk imports and an %11 speedup for GI method calls. Additional notes on performance from comment #10 are still valid.
Created attachment 249452 [details] [review] Replace Python VFunc descriptor directly with PyGIVFuncInfo Add tp_getdesc (__get__) to PyGIVFuncInfo to allow the object to be used directly as a callable descriptor. This piggy backs off the added support for functions and constructors in previous patches.
Comment on attachment 249045 [details] [review] Move doc string generator into separate module This is a nice factorization and should not have any performance impact, LGTM.
Comment on attachment 249097 [details] [review] Remove redundant info.get_name calls Nice!
Review of attachment 249320 [details] [review]: Looks good otherwise, please push with the missing space fixed. Thanks! ::: gi/pygi-info.c @@ +210,3 @@ + static PyObject *docstr; + if (docstr == NULL) { + docstr= PYGLIB_PyUnicode_InternFromString("__doc__"); Only a tiny nitpick here, missing space before "=".
For the remaining two, I'm honestly not feeling qualified enough to sensibly review them. I verified that they work fine for both py2 and py3 though, so if you feel good about them, please let's get them into 3.9.5 for wider testing. Please do consider running check.valgrind before/after and ensure that this doesn't introduce more leaks. (NB you'll need to run with PYTHON=... pointing to a custom python build with --enable-valgrind, as the Ubuntu packages don't do that). Thanks!
Attachment 249045 [details] pushed as 6fdde25 - Move doc string generator into separate module Attachment 249097 [details] pushed as 6b36fbe - Remove redundant info.get_name calls Attachment 249103 [details] pushed as ea19440 - Merge method and constructor setup Attachment 249320 [details] pushed as bec0b54 - Add common attribute accessors to PyGIBaseInfo
Running valgrind was definitely a good call. While these patches by themself didn't have any leaks, the new setup which removes layers of Python partials exposed problems with how we hack vfunc return values. I've fixed this in a prior commit: https://git.gnome.org/browse/pygobject/commit/?id=2339e030e4d Attachment 249440 [details] pushed as 35f79b2 - Add callable and descriptor protocols to PyGICallableInfo Attachment 249452 [details] pushed as 91c4982 - Replace Python VFunc descriptor directly with PyGIVFuncInfo