Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GH-103082: Filter LINE events in VM, to simplify tool implementation. #104387

Merged
merged 8 commits into from
May 12, 2023
Merged

GH-103082: Filter LINE events in VM, to simplify tool implementation. #104387

merged 8 commits into from
May 12, 2023

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented May 11, 2023 •
edited by bedevere-bot
Loading

When monitoring LINE events, instrument all instructions that can have a predecessor on a different line.
Then check that the a new line has been hit in the instrumentation code.
This brings the behavior closer to that of 3.11, simplifying implementation and porting of tools.

Fixes #103471
Fixes #104239

@markshannon
Filter LINE events in VM, to simplify tool implementation.
4190fe7 
When monitoring LINE events, instrument all instructions that can have a predecessor on a different line.
Then check that the a new line has been hit in the instrumentation code.
This brings the behavior closer to that of 3.11, simplifying implementation and porting of tools.
@markshannon
Merge branch 'main' into instrument-all-possible-lines
6e8c98f
@markshannon
Add news
cc62dcb
@markshannon
Mark _PyInstrumentation_DISABLE as a legal static value.
5418580
@markshannon
Merge branch 'main' into instrument-all-possible-lines
1da7272
@markshannon
Address review comments.
e76f241
@markshannon
Merge branch 'main' into instrument-all-possible-lines
b8d6799
@markshannon
Add a couple more comments.
f9cf3db
@bedevere-bot bedevere-bot mentioned this pull request May 11, 2023
Closed
4 tasks
@markshannon markshannon mentioned this pull request May 11, 2023
Merged
@brandtbucher brandtbucher self-requested a review May 11, 2023 18:02
brandtbucher
brandtbucher approved these changes May 11, 2023
View reviewed changes
Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I feel like I have a pretty good grasp of what's going on here, and it looks good.

Just a couple of suggestions where you might add some comments in areas I struggled to finally understand, as well as a couple of questions to clarify some things I couldn't figure out:

Lib/test/test_pdb.py Show resolved Hide resolved
Python/instrumentation.c Show resolved Hide resolved
Python/instrumentation.c Show resolved Hide resolved
Python/ceval.c Show resolved Hide resolved
Python/instrumentation.c Outdated Show resolved Hide resolved
Python/instrumentation.c Show resolved Hide resolved
Python/instrumentation.c Show resolved Hide resolved
Python/ceval.c Show resolved Hide resolved
Python/instrumentation.c Show resolved Hide resolved
@markshannon markshannon added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label May 12, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @markshannon for commit f9cf3db 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label May 12, 2023
@markshannon
Copy link
Member Author

The failure on RHEL7 Refleaks PR also appears on main.

$ ./python -m test -R 3:3 test_typing
0:00:00 load avg: 1.97 Run tests sequentially
0:00:00 load avg: 1.97 [1/1] test_typing
beginning 6 repetitions
123456
.test test_typing failed -- Traceback (most recent call last):
  File "/home/mark/cpython/Lib/test/test_typing.py", line 6007, in test_bytestring
    with self.assertWarns(DeprecationWarning):
AssertionError: DeprecationWarning not triggered

test_typing failed (1 failure)

== Tests result: FAILURE ==

1 test failed:
    test_typing

Total duration: 546 ms
Tests result: FAILURE

The doc test failure is also unrelated, I think.

@markshannon markshannon merged commit 45f5aa8 into python:main May 12, 2023
@markshannon markshannon deleted the instrument-all-possible-lines branch May 12, 2023 11:21
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Debian root 3.x has failed when building commit 45f5aa8.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/345/builds/4764) and take a look at the build logs.
  4. Check if the failure is related to this commit (45f5aa8) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/345/builds/4764

Failed tests:

  • test__xxsubinterpreters

Summary of the results of the build (if available):

==

Click to see traceback logs 
remote: Enumerating objects: 60, done.        
remote: Counting objects:   1% (1/60)        
remote: Counting objects:   3% (2/60)        
remote: Counting objects:   5% (3/60)        
remote: Counting objects:   6% (4/60)        
remote: Counting objects:   8% (5/60)        
remote: Counting objects:  10% (6/60)        
remote: Counting objects:  11% (7/60)        
remote: Counting objects:  13% (8/60)        
remote: Counting objects:  15% (9/60)        
remote: Counting objects:  16% (10/60)        
remote: Counting objects:  18% (11/60)        
remote: Counting objects:  20% (12/60)        
remote: Counting objects:  21% (13/60)        
remote: Counting objects:  23% (14/60)        
remote: Counting objects:  25% (15/60)        
remote: Counting objects:  26% (16/60)        
remote: Counting objects:  28% (17/60)        
remote: Counting objects:  30% (18/60)        
remote: Counting objects:  31% (19/60)        
remote: Counting objects:  33% (20/60)        
remote: Counting objects:  35% (21/60)        
remote: Counting objects:  36% (22/60)        
remote: Counting objects:  38% (23/60)        
remote: Counting objects:  40% (24/60)        
remote: Counting objects:  41% (25/60)        
remote: Counting objects:  43% (26/60)        
remote: Counting objects:  45% (27/60)        
remote: Counting objects:  46% (28/60)        
remote: Counting objects:  48% (29/60)        
remote: Counting objects:  50% (30/60)        
remote: Counting objects:  51% (31/60)        
remote: Counting objects:  53% (32/60)        
remote: Counting objects:  55% (33/60)        
remote: Counting objects:  56% (34/60)        
remote: Counting objects:  58% (35/60)        
remote: Counting objects:  60% (36/60)        
remote: Counting objects:  61% (37/60)        
remote: Counting objects:  63% (38/60)        
remote: Counting objects:  65% (39/60)        
remote: Counting objects:  66% (40/60)        
remote: Counting objects:  68% (41/60)        
remote: Counting objects:  70% (42/60)        
remote: Counting objects:  71% (43/60)        
remote: Counting objects:  73% (44/60)        
remote: Counting objects:  75% (45/60)        
remote: Counting objects:  76% (46/60)        
remote: Counting objects:  78% (47/60)        
remote: Counting objects:  80% (48/60)        
remote: Counting objects:  81% (49/60)        
remote: Counting objects:  83% (50/60)        
remote: Counting objects:  85% (51/60)        
remote: Counting objects:  86% (52/60)        
remote: Counting objects:  88% (53/60)        
remote: Counting objects:  90% (54/60)        
remote: Counting objects:  91% (55/60)        
remote: Counting objects:  93% (56/60)        
remote: Counting objects:  95% (57/60)        
remote: Counting objects:  96% (58/60)        
remote: Counting objects:  98% (59/60)        
remote: Counting objects: 100% (60/60)        
remote: Counting objects: 100% (60/60), done.        
remote: Compressing objects:   3% (1/29)        
remote: Compressing objects:   6% (2/29)        
remote: Compressing objects:  10% (3/29)        
remote: Compressing objects:  13% (4/29)        
remote: Compressing objects:  17% (5/29)        
remote: Compressing objects:  20% (6/29)        
remote: Compressing objects:  24% (7/29)        
remote: Compressing objects:  27% (8/29)        
remote: Compressing objects:  31% (9/29)        
remote: Compressing objects:  34% (10/29)        
remote: Compressing objects:  37% (11/29)        
remote: Compressing objects:  41% (12/29)        
remote: Compressing objects:  44% (13/29)        
remote: Compressing objects:  48% (14/29)        
remote: Compressing objects:  51% (15/29)        
remote: Compressing objects:  55% (16/29)        
remote: Compressing objects:  58% (17/29)        
remote: Compressing objects:  62% (18/29)        
remote: Compressing objects:  65% (19/29)        
remote: Compressing objects:  68% (20/29)        
remote: Compressing objects:  72% (21/29)        
remote: Compressing objects:  75% (22/29)        
remote: Compressing objects:  79% (23/29)        
remote: Compressing objects:  82% (24/29)        
remote: Compressing objects:  86% (25/29)        
remote: Compressing objects:  89% (26/29)        
remote: Compressing objects:  93% (27/29)        
remote: Compressing objects:  96% (28/29)        
remote: Compressing objects: 100% (29/29)        
remote: Compressing objects: 100% (29/29), done.        
remote: Total 31 (delta 29), reused 3 (delta 2), pack-reused 0        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to '45f5aa8fc73acf516071d52ef8213532f0381316'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 45f5aa8fc7 GH-103082: Filter LINE events in VM, to simplify tool implementation. (GH-104387)
Switched to and reset branch 'main'

configure: WARNING: pkg-config is missing. Some dependencies may not be detected correctly.

Fatal Python error: drop_gil: PyMUTEX_LOCK(gil->switch_mutex) failed
Python runtime state: initialized

Thread 0x00007f090e821280 (most recent call first):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test__xxsubinterpreters.py", line 80 in clean_up_interpreters
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test__xxsubinterpreters.py", line 88 in tearDown
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/case.py", line 594 in _callTearDown
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/case.py", line 637 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/case.py", line 690 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 122 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 122 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 122 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/runner.py", line 240 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/__init__.py", line 1106 in _run_suite
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/__init__.py", line 1232 in run_unittest
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 294 in _test_module
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 330 in _runtest_inner2
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 373 in _runtest_inner
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 248 in _runtest
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 278 in runtest
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 365 in rerun_failed_tests
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 783 in _main
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 738 in main
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 802 in main
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/__main__.py", line 2 in <module>
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/runpy.py", line 88 in _run_code
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/runpy.py", line 198 in _run_module_as_main
Fatal Python error: Aborted

Thread 0x00007f090e821280 (most recent call first):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test__xxsubinterpreters.py", line 80 in clean_up_interpreters
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test__xxsubinterpreters.py", line 88 in tearDown
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/case.py", line 594 in _callTearDown
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/case.py", line 637 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/case.py", line 690 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 122 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 122 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 122 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/runner.py", line 240 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/__init__.py", line 1106 in _run_suite
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/__init__.py", line 1232 in run_unittest
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 294 in _test_module
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 330 in _runtest_inner2
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 373 in _runtest_inner
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 248 in _runtest
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 278 in runtest
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 365 in rerun_failed_tests
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 783 in _main
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 738 in main
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 802 in main
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/__main__.py", line 2 in <module>
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/runpy.py", line 88 in _run_code
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/runpy.py", line 198 in _run_module_as_main

Extension modules: _testcapi, _xxsubinterpreters (total: 2)
Aborted
make: *** [Makefile:1956: buildbottest] Error 134

Cannot open file '/root/buildarea/3.x.angelico-debian-amd64/build/test-results.xml' for upload

carljm added a commit to carljm/cpython that referenced this pull request May 12, 2023
@carljm
Merge branch 'main' into supermon
8efd7c6 
* main:
  pythongh-91896: Fixup some docs issues following ByteString deprecation (python#104422)
  pythonGH-104371: check return value of calling `mv.release` (python#104417)
  pythongh-104415: Fix refleak tests for `typing.ByteString` deprecation (python#104416)
  pythonGH-86275: Implementation of hypothesis stubs for property-based tests, with zoneinfo tests (python#22863)
  pythonGH-103082: Filter LINE events in VM, to simplify tool implementation. (pythonGH-104387)
  pythongh-93649: Split gc- and allocation tests from _testcapimodule.c (pythonGH-1