This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Fix gdb plugin for new sets dummy object
Type: behavior Stage: resolved
Components: Demos and Tools, Interpreter Core Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: pitrou Nosy List: dmalcolm, larry, pitrou, python-dev, rhettinger
Priority: release blocker Keywords: patch

Created on 2013-08-17 19:18 by pitrou, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
gdb_sets_repr.patch pitrou, 2013-08-17 19:18 review
fix_dummy.diff rhettinger, 2013-08-23 12:51 Check repr() instead of exact match review
gdb_sets_repr2.patch pitrou, 2013-08-23 18:28 review
Messages (18)
msg195509 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) (Python committer) Date: 2013-08-19 17:53
Antoine's patch looks reasonable to me, FWIW.
msg195659 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) 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) * (Python committer) 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) (Python triager) 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) * (Python committer) Date: 2013-08-21 15:47
Antoine, can you please verify the gdb plug-in is working again?
msg195830 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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) * (Python committer) Date: 2013-08-22 04:01
Thank you.
msg195961 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) (Python triager) 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) * (Python committer) Date: 2013-08-24 19:08
Well, it should be ok now (at least now the test passes here).
History
Date User Action Args
2022-04-11 14:57:49adminsetgithub: 62972
2013-08-24 19:08:56pitrousetstatus: open -> closed

messages: + msg196091
stage: patch review -> resolved
2013-08-24 19:07:16python-devsetmessages: + msg196089
2013-08-24 19:03:35pitrousetmessages: + msg196088
2013-08-23 18:28:43pitrousetfiles: + gdb_sets_repr2.patch

messages: + msg195997
2013-08-23 13:42:02pitrousetmessages: + msg195969
2013-08-23 12:51:01rhettingersetfiles: + fix_dummy.diff

messages: + msg195965
2013-08-23 12:22:38pitrousetmessages: + msg195963
2013-08-23 12:11:24rhettingersetstatus: closed -> open
assignee: rhettinger -> pitrou
messages: + msg195961
2013-08-22 04:01:27rhettingersetstatus: open -> closed
resolution: fixed
messages: + msg195857
2013-08-21 22:36:46pitrousetmessages: + msg195830
2013-08-21 15:47:05rhettingersetmessages: + msg195799
2013-08-21 05:29:25rhettingersetmessages: - msg195720
2013-08-21 05:28:41python-devsetnosy: + python-dev
messages: + msg195745
2013-08-20 22:16:27rhettingersetmessages: + msg195720
2013-08-20 08:19:59pitrousetmessages: + msg195680
2013-08-19 19:57:44rhettingersetmessages: + msg195659
2013-08-19 18:03:04rhettingersetassignee: rhettinger
2013-08-19 17:53:26dmalcolmsetmessages: + msg195654
2013-08-19 17:26:12pitrousetmessages: + msg195649
2013-08-18 18:31:08pitrousetmessages: + msg195574
2013-08-17 19:22:35pitrousettitle: Fix test_gdb for new sets dummy object -> Fix gdb plugin for new sets dummy object
2013-08-17 19:18:25pitrousetnosy: + rhettinger, dmalcolm
2013-08-17 19:18:11pitroucreate