msg195509 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-08-17 19:18 |
Changeset 2c9a2b588a89 broke the pretty-printing of sets by the gdb plugin.
Here is a temptative patch. It works, but I don't know enough to know whether that's the right coding style for a gdb plugin. Dave?
|
msg195574 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-08-18 18:31 |
See also the python-dev thread about other drawbacks of the new dummy approach: http://mail.python.org/pipermail/python-dev/2013-August/128025.html
|
msg195649 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-08-19 17:26 |
Raymond, if you don't have time for this issue, I'd prefer your patch to be reverted. It hasn't been proven to accelerate any benchmark, and the problems it triggered need solving.
|
msg195654 - (view) |
Author: Dave Malcolm (dmalcolm) |
Date: 2013-08-19 17:53 |
Antoine's patch looks reasonable to me, FWIW.
|
msg195659 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2013-08-19 19:57 |
This is a bummer. The internal comments made the claim that any python object would suffice. The use of a unicode object was unfortunate because it isn't guaranteed to be unique (i.e. a user can legitimately store "<dummy>" as a valid member of a set). As a consequence, the tight loops for the hash table lookups had to add special case checks for the dummy variables. Eliminating those checks made the generated code shorter and faster. It also paved the way for a future optimization for non-debug builds to eliminate all non-essential increfs and decrefs to the dummy object.
In other words, we're paying a price for the dummy object being printable as a string.
Antoine, thanks to the link to the python-dev discussion. I hope we can come with a solution that doesn't involve going back to unicode objects.
|
msg195680 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-08-20 08:19 |
> In other words, we're paying a price for the dummy object being
> printable as a string.
Which is pointless, I agree.
> Antoine, thanks to the link to the python-dev discussion. I hope we
> can come with a solution that doesn't involve going back to unicode
> objects.
Re-using PyDict's dummy object would be a solution, AFAICT?
|
msg195745 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-08-21 05:28 |
New changeset be29efa60b68 by Raymond Hettinger in branch 'default':
Issue 18772: Restore set dummy object back to unicode and restore the identity checks in lookkey().
http://hg.python.org/cpython/rev/be29efa60b68
|
msg195799 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2013-08-21 15:47 |
Antoine, can you please verify the gdb plug-in is working again?
|
msg195830 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-08-21 22:36 |
> Antoine, can you please verify the gdb plug-in is working again?
test_gdb works fine here, yes.
|
msg195857 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2013-08-22 04:01 |
Thank you.
|
msg195961 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2013-08-23 12:11 |
I just applied the dict version of dummy to sets in http://hg.python.org/cpython/rev/f0202c3daa7a
and it unexpectedly broke test_gdb again:
FAIL: test_sets (test.test_gdb.PrettyPrintTests)
Verify the pretty-printing of sets
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/shager/cpython-buildarea/3.x.edelsohn-powerlinux-ppc64/build/Lib/test/test_gdb.py", line 319, in test_sets
self.assertEqual(gdb_repr, "{'b'}")
AssertionError: "{<<dummy key> type at remote 0x103b35c0>, 'b'}" != "{'b'}"
- {<<dummy key> type at remote 0x103b35c0>, 'b'}
+ {'b'}
Antoine, can you see what the issue is? Why does this show as the dummy key type instead of being recognized as the object returned by _PySet_Dummy? I'm not sure why this works for dicts, but not for sets.
|
msg195963 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-08-23 12:22 |
Checking for dummies in dicts in python-gdb.py is implicit (see PyDictObjectPtr.iteritems()): entries whose value is NULL are not printed.
Set entries do not have values, so instead pySetObjectPtr.write_repr() uses a hack with the repr() of the key to check for dummies. Instead I think we should re-use the approach in my patch: publish the dummy object as a (private) C API so that python-gdb.py can simply compare the key pointer by equality.
|
msg195965 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2013-08-23 12:51 |
Attaching a patch, but I don't have a way to test it.
|
msg195969 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-08-23 13:42 |
Checking repr() perpetuates the original hack so, why it may work, I think it's better if we take the opportunity to solve it cleanly.
I'll try to take a look tonight (I suppose you're under Windows?).
|
msg195997 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-08-23 18:28 |
Attached patch solves the issue and removes the repr()-based hack in the gdb plugin.
|
msg196088 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-08-24 19:03 |
Since nobody seems to object to the patch, I'm commit it :)
|
msg196089 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-08-24 19:07 |
New changeset 2f1bac39565a by Antoine Pitrou in branch 'default':
Issue #18772: fix the gdb plugin after the set implementation changes
http://hg.python.org/cpython/rev/2f1bac39565a
|
msg196091 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-08-24 19:08 |
Well, it should be ok now (at least now the test passes here).
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:49 | admin | set | github: 62972 |
2013-08-24 19:08:56 | pitrou | set | status: open -> closed
messages:
+ msg196091 stage: patch review -> resolved |
2013-08-24 19:07:16 | python-dev | set | messages:
+ msg196089 |
2013-08-24 19:03:35 | pitrou | set | messages:
+ msg196088 |
2013-08-23 18:28:43 | pitrou | set | files:
+ gdb_sets_repr2.patch
messages:
+ msg195997 |
2013-08-23 13:42:02 | pitrou | set | messages:
+ msg195969 |
2013-08-23 12:51:01 | rhettinger | set | files:
+ fix_dummy.diff
messages:
+ msg195965 |
2013-08-23 12:22:38 | pitrou | set | messages:
+ msg195963 |
2013-08-23 12:11:24 | rhettinger | set | status: closed -> open assignee: rhettinger -> pitrou messages:
+ msg195961
|
2013-08-22 04:01:27 | rhettinger | set | status: open -> closed resolution: fixed messages:
+ msg195857
|
2013-08-21 22:36:46 | pitrou | set | messages:
+ msg195830 |
2013-08-21 15:47:05 | rhettinger | set | messages:
+ msg195799 |
2013-08-21 05:29:25 | rhettinger | set | messages:
- msg195720 |
2013-08-21 05:28:41 | python-dev | set | nosy:
+ python-dev messages:
+ msg195745
|
2013-08-20 22:16:27 | rhettinger | set | messages:
+ msg195720 |
2013-08-20 08:19:59 | pitrou | set | messages:
+ msg195680 |
2013-08-19 19:57:44 | rhettinger | set | messages:
+ msg195659 |
2013-08-19 18:03:04 | rhettinger | set | assignee: rhettinger |
2013-08-19 17:53:26 | dmalcolm | set | messages:
+ msg195654 |
2013-08-19 17:26:12 | pitrou | set | messages:
+ msg195649 |
2013-08-18 18:31:08 | pitrou | set | messages:
+ msg195574 |
2013-08-17 19:22:35 | pitrou | set | title: Fix test_gdb for new sets dummy object -> Fix gdb plugin for new sets dummy object |
2013-08-17 19:18:25 | pitrou | set | nosy:
+ rhettinger, dmalcolm
|
2013-08-17 19:18:11 | pitrou | create | |