diff mbox

[python] Optimize python_string_to_host_string() for Python 2.

Message ID 1421217481-18204-1-git-send-email-agentzh@gmail.com
State New, archived
Headers

Commit Message

Yichun Zhang (agentzh) Jan. 14, 2015, 6:38 a.m. UTC 
  Hi,

I have been writing quite some advanced GDB Python tools for LuaJIT
and NGINX in the nginx-gdb-utils project [1] and have been suffering
from serious performance issues even for relatively small working sets.

I have noticed from a typical on-CPU C-land Flame Graph [2]
for my tools that python_string_to_host_string() is painfully slow
when using Python 2 (2.7 to be more specific) due to the
expensive and meaningless unicode conversions there (involved with
hot function calls like utf_8_decode).

With the following patch, my most complicated Python tools
finally have comparable performance between Python 2 and
Python 3. In the case of Python 2, some real-world Python scripts'
overall speedup can be as big as 34% (for my "lgcpath" command [3])
or even 57% (for my "lgcstat" command [4]). And from the new Flame
Graph [5], we can see that the corresponding function frames are
indeed gone.

Comments are welcome!

[1] https://github.com/openresty/nginx-gdb-utils#readme
[2] http://agentzh.org/misc/flamegraph/gdb-py-lgcstat-2015-0113.svg
[3] https://github.com/openresty/nginx-gdb-utils#lgcpath
[4] https://github.com/openresty/nginx-gdb-utils#lgcstat
[5] http://agentzh.org/misc/flamegraph/patched-gdb-py-lgcstat-2015-0113.svg

gdb/ChangeLog:

    * python/py-utils.c (python_string_to_host_string): use
    xstrdup directly for Python 2.
    * python/py-utils.c (gdbpy_obj_to_string): use
    python_string_to_host_string for both Python 2 and 3.
---
 gdb/python/py-utils.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Yichun Zhang (agentzh) Jan. 19, 2015, 10:08 p.m. UTC | #1 
Hello!

On Tue, Jan 13, 2015 at 10:38 PM, Yichun Zhang (agentzh) wrote:
> With the following patch, my most complicated Python tools
> finally have comparable performance between Python 2 and
> Python 3. In the case of Python 2, some real-world Python scripts'
> overall speedup can be as big as 34% (for my "lgcpath" command [3])
> or even 57% (for my "lgcstat" command [4]). And from the new Flame
> Graph [5], we can see that the corresponding function frames are
> indeed gone.
>
> Comments are welcome!
>

Does the silence in the last 6 days mean that this patch is completely wrong?

Actually I have a few more patches to submit to optimize this
gdb/python part even further. The silence here makes me hesitate ;)

Please let me know if I'm doing something wrong here. Very much appreciated!

Thanks!
-agentzh
Phil Muldoon Jan. 20, 2015, 11:13 a.m. UTC | #2 
On 19/01/15 22:08, Yichun Zhang (agentzh) wrote:
> Hello!
>
> On Tue, Jan 13, 2015 at 10:38 PM, Yichun Zhang (agentzh) wrote:
>> With the following patch, my most complicated Python tools
>> finally have comparable performance between Python 2 and
>> Python 3. In the case of Python 2, some real-world Python scripts'
>> overall speedup can be as big as 34% (for my "lgcpath" command [3])
>> or even 57% (for my "lgcstat" command [4]). And from the new Flame
>> Graph [5], we can see that the corresponding function frames are
>> indeed gone.
>>
>> Comments are welcome!
>>
>
> Does the silence in the last 6 days mean that this patch is completely wrong?

Not at all.  I think the post Christmas catch-up has affected patch
review and there is some lag.  I am sure the maintainers will review
your patch soon.

> Actually I have a few more patches to submit to optimize this
> gdb/python part even further. The silence here makes me hesitate ;)

Please submit them. There is no hesitation on commenting on patches
constructively. For my part, I find no issue with people indicating
what might be wrong (or right) with my patches!

> Please let me know if I'm doing something wrong here. Very much appreciated!

You are doing nothing wrong.  Keep pinging your patches. The usual
wait time for pinging a patch is seven days.

Cheers

Phil
Joel Brobecker Jan. 20, 2015, 1:13 p.m. UTC | #3 
In addition to Phil's answer (Thank You, Phil!), feel free to ping us
about pending patches like you just did on a weekly basis. We're all
volunteers, so we do not always have the time to review right away.
Yichun Zhang (agentzh) Jan. 20, 2015, 10:18 p.m. UTC | #4 
Hello!

I see. Thank you for the replies, Phil and Joel!

Oh yeah, as the maintainer of many opensource projects myself, I
totally understand the situation especially given the relatively high
traffic volume on this list :)

That's cool and I'll keep publishing more patches here :)

Best regards,
-agentzh
Yichun Zhang (agentzh) Feb. 1, 2015, 11:33 p.m. UTC | #5 
Hi

On Tue, Jan 13, 2015 at 10:38 PM, Yichun Zhang (agentzh) wrote:
> With the following patch, my most complicated Python tools
> finally have comparable performance between Python 2 and
> Python 3. In the case of Python 2, some real-world Python scripts'
> overall speedup can be as big as 34% (for my "lgcpath" command [3])
> or even 57% (for my "lgcstat" command [4]). And from the new Flame
> Graph [5], we can see that the corresponding function frames are
> indeed gone.

ping :)

-agentzh
Yichun Zhang (agentzh) Feb. 22, 2015, 6:01 p.m. UTC | #6 
Hello!

On Tue, Jan 13, 2015 at 10:38 PM, Yichun Zhang (agentzh) wrote:
> With the following patch, my most complicated Python tools
> finally have comparable performance between Python 2 and
> Python 3. In the case of Python 2, some real-world Python scripts'
> overall speedup can be as big as 34% (for my "lgcpath" command [3])
> or even 57% (for my "lgcstat" command [4]). And from the new Flame
> Graph [5], we can see that the corresponding function frames are
> indeed gone.
>

ping

Regards,
-agentzh
Joel Brobecker March 3, 2015, 4:11 p.m. UTC | #7 
Hello,

First of all, sorry about the delay in review this. I was hoping
that someone more knowledgeable than I would be able to review it.
Thanks for ping us, and for your patience!

> I have been writing quite some advanced GDB Python tools for LuaJIT
> and NGINX in the nginx-gdb-utils project [1] and have been suffering
> from serious performance issues even for relatively small working sets.
> 
> I have noticed from a typical on-CPU C-land Flame Graph [2]
> for my tools that python_string_to_host_string() is painfully slow
> when using Python 2 (2.7 to be more specific) due to the
> expensive and meaningless unicode conversions there (involved with
> hot function calls like utf_8_decode).
[...]
> gdb/ChangeLog:
> 
>     * python/py-utils.c (python_string_to_host_string): use
>     xstrdup directly for Python 2.
>     * python/py-utils.c (gdbpy_obj_to_string): use
>     python_string_to_host_string for both Python 2 and 3.

I believe you, but being a fairly complete dummy in this area,
can you explain why the unicode conversion is meaningless with
Python 2? In fact, if you have that explanation, I think it would
be useful to have that as a comment inside the Python-2 part of
python_string_to_host_string.

Thanks,
diff mbox

Patch

diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 58a5934..ed7be3b 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -209,6 +209,7 @@  python_string_to_target_python_string (PyObject *obj)
 char *
 python_string_to_host_string (PyObject *obj)
 {
+#ifdef IS_PY3K
   PyObject *str;
   char *result;
 
@@ -219,6 +220,9 @@  python_string_to_host_string (PyObject *obj)
   result = unicode_to_encoded_string (str, host_charset ());
   Py_DECREF (str);
   return result;
+#else
+  return xstrdup (PyString_AsString (obj));
+#endif
 }
 
 /* Return true if OBJ is a Python string or unicode object, false
@@ -245,12 +249,7 @@  gdbpy_obj_to_string (PyObject *obj)
 
   if (str_obj != NULL)
     {
-#ifdef IS_PY3K
       char *msg = python_string_to_host_string (str_obj);
-#else
-      char *msg = xstrdup (PyString_AsString (str_obj));
-#endif
-
       Py_DECREF (str_obj);
       return msg;
     }