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.

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

gh-112536: Add --tsan test for reasonable TSAN execution times. #116601

Merged
merged 14 commits into from
Mar 15, 2024

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Mar 11, 2024

@@ -0,0 +1,3 @@
## reference: https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions
Copy link
Member Author

Choose a reason for hiding this comment

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

I am going to update the list of supressions.txt when working on the CI things.

'test_capi',
'test_code',
'test_compileall',
'test_concurrent_futures',
Copy link
Member Author

Choose a reason for hiding this comment

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

==14799==ThreadSanitizer: starting new threads after multi-threaded fork is not supported. Dying (set die_after_fork=0 to override)

@corona10
Copy link
Member Author

@colesbury
We may need to decide which tests should be included.

  • Successful tests only?
  • Add failed tests too but update supressions.txt for this?

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@corona10
Copy link
Member Author

We may need to decide which tests should be included.

Anyway, we should exclude some of test from the list since it takes over 1 hours :)

@corona10
Copy link
Member Author

corona10 commented Mar 11, 2024

@colesbury @swtaarrs

I think that this setting could be the minimum setup for adding TSAN in CI.
We can add more tests if needed. Here is the local execution result from my Azure VM Ubuntu.
(Standard D4s v3, ubuntu 22.04)
And I recommend not using --with-pydebug. It's super slow, but let's see if it will be an acceptable execution time on GA.
(With separated PR)

$ ./python -m test --tsan --timeout=1200
Using random seed: 1515375220
0:00:00 load avg: 0.79 Run 17 tests in parallel using 6 worker processes (timeout: 20 min, worker timeout: 25 min)
0:00:30 load avg: 2.84 running (6): test_code (30.1 sec), test_enum (30.1 sec), test_functools (30.1 sec), test_httpservers (30.1 sec), test_imaplib (30.0 sec), test_importlib (30.0 sec)
0:00:38 load avg: 3.33 [ 1/17] test_code passed (38.4 sec) -- running (5): test_enum (38.4 sec), test_functools (38.4 sec), test_httpservers (38.4 sec), test_imaplib (38.4 sec), test_importlib (38.4 sec)
0:00:57 load avg: 3.91 [ 2/17] test_functools passed (57.4 sec) -- running (4): test_enum (57.4 sec), test_httpservers (57.4 sec), test_imaplib (57.4 sec), test_importlib (57.4 sec)
0:01:05 load avg: 4.23 [ 3/17] test_imaplib passed (1 min 5 sec) -- running (3): test_enum (1 min 5 sec), test_httpservers (1 min 5 sec), test_importlib (1 min 5 sec)
0:01:36 load avg: 5.06 running (6): test_io (57.2 sec), test_enum (1 min 35 sec), test_logging (38.2 sec), test_httpservers (1 min 35 sec), test_tuple (30.1 sec), test_importlib (1 min 35 sec)
0:01:43 load avg: 5.28 [ 4/17] test_httpservers passed (1 min 43 sec) -- running (5): test_io (1 min 4 sec), test_enum (1 min 43 sec), test_logging (45.8 sec), test_tuple (37.7 sec), test_importlib (1 min 43 sec)
0:02:13 load avg: 5.58 running (6): test_io (1 min 34 sec), test_enum (2 min 13 sec), test_logging (1 min 15 sec), test_list (30.0 sec), test_tuple (1 min 7 sec), test_importlib (2 min 13 sec)
0:02:43 load avg: 5.81 running (6): test_io (2 min 4 sec), test_enum (2 min 43 sec), test_logging (1 min 45 sec), test_list (1 min), test_tuple (1 min 37 sec), test_importlib (2 min 43 sec)
0:03:13 load avg: 5.94 running (6): test_io (2 min 34 sec), test_enum (3 min 13 sec), test_logging (2 min 15 sec), test_list (1 min 30 sec), test_tuple (2 min 7 sec), test_importlib (3 min 13 sec)
0:03:13 load avg: 5.94 [ 5/17] test_tuple passed (2 min 8 sec) -- running (5): test_io (2 min 35 sec), test_enum (3 min 13 sec), test_logging (2 min 16 sec), test_list (1 min 30 sec), test_importlib (3 min 13 sec)
0:03:23 load avg: 5.95 [ 6/17] test_list passed (1 min 39 sec) -- running (4): test_io (2 min 44 sec), test_enum (3 min 22 sec), test_logging (2 min 25 sec), test_importlib (3 min 22 sec)
0:03:53 load avg: 5.95 running (6): test_io (3 min 14 sec), test_enum (3 min 52 sec), test_logging (2 min 55 sec), test_ssl (30.0 sec), test_dict (39.1 sec), test_importlib (3 min 52 sec)
0:04:03 load avg: 6.04 [ 7/17] test_logging passed (3 min 5 sec) -- running (5): test_io (3 min 24 sec), test_enum (4 min 2 sec), test_ssl (40.1 sec), test_dict (49.2 sec), test_importlib (4 min 2 sec)
/home/corona10/cpython/Lib/test/test_logging.py:783: DeprecationWarning: This process (pid=63080) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = os.fork()
/home/corona10/cpython/Lib/multiprocessing/popen_fork.py:66: DeprecationWarning: This process (pid=63080) is multi-threaded, use of fork() may lead to deadlocks in the child.
  self.pid = os.fork()
0:04:16 load avg: 5.96 [ 8/17] test_dict passed (1 min 2 sec) -- running (4): test_io (3 min 37 sec), test_enum (4 min 16 sec), test_ssl (53.7 sec), test_importlib (4 min 16 sec)
0:04:39 load avg: 6.41 [ 9/17] test_syslog passed (35.9 sec) -- running (4): test_io (4 min), test_enum (4 min 38 sec), test_ssl (1 min 15 sec), test_importlib (4 min 38 sec)
0:04:57 load avg: 6.47 [10/17] test_thread passed (40.5 sec) -- running (4): test_io (4 min 18 sec), test_enum (4 min 56 sec), test_ssl (1 min 34 sec), test_importlib (4 min 56 sec)
0:05:06 load avg: 6.56 [11/17] test_threadedtempfile passed -- running (4): test_io (4 min 27 sec), test_enum (5 min 6 sec), test_ssl (1 min 43 sec), test_importlib (5 min 6 sec)
0:05:32 load avg: 6.49 [12/17] test_ssl passed (2 min 9 sec) -- running (4): test_io (4 min 53 sec), test_enum (5 min 32 sec), test_threading_local (35.6 sec), test_importlib (5 min 32 sec)
0:05:36 load avg: 6.37 [13/17] test_threadsignals passed -- running (4): test_io (4 min 57 sec), test_enum (5 min 35 sec), test_threading_local (38.9 sec), test_importlib (5 min 35 sec)
0:05:50 load avg: 6.07 [14/17] test_enum passed (5 min 49 sec) -- running (3): test_io (5 min 11 sec), test_threading_local (53.2 sec), test_importlib (5 min 50 sec)
0:05:50 load avg: 6.07 [15/17] test_threading_local passed (53.3 sec) -- running (2): test_io (5 min 11 sec), test_importlib (5 min 50 sec)
0:06:20 load avg: 4.60 running (2): test_io (5 min 41 sec), test_importlib (6 min 20 sec)
0:06:50 load avg: 4.19 running (2): test_io (6 min 11 sec), test_importlib (6 min 50 sec)
0:07:20 load avg: 3.81 running (2): test_io (6 min 41 sec), test_importlib (7 min 20 sec)
0:07:50 load avg: 3.27 running (2): test_io (7 min 11 sec), test_importlib (7 min 50 sec)
0:08:07 load avg: 2.99 [16/17] test_importlib passed (8 min 7 sec) -- running (1): test_io (7 min 28 sec)
0:08:37 load avg: 2.20 running (1): test_io (7 min 58 sec)
0:09:07 load avg: 1.73 running (1): test_io (8 min 28 sec)
0:09:37 load avg: 1.52 running (1): test_io (8 min 58 sec)
0:09:47 load avg: 1.36 [17/17] test_io passed (9 min 8 sec)

== Tests result: SUCCESS ==

All 17 tests OK.

Total duration: 9 min 47 sec
Total tests: run=4,167 skipped=59
Total test files: run=17/17
Result: SUCCESS

@corona10 corona10 requested a review from swtaarrs March 11, 2024 17:04
Doc/library/test.rst Outdated Show resolved Hide resolved
Lib/test/libregrtest/main.py Outdated Show resolved Hide resolved
@@ -333,6 +334,8 @@ def _create_parser():
help='enable Profile Guided Optimization (PGO) training')
group.add_argument('--pgo-extended', action='store_true',
help='enable extended PGO training (slower training)')
group.add_argument('--tsan', dest='tsan', action='store_true',
help='enable TSAN test')
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain what "TSAN test" means here? From the rest of the diff it sounds like it's "Tests that pass in a reasonable amount of time with TSAN enabled", but that's not clear from this help string.

Doc/library/test.rst Outdated Show resolved Hide resolved
Lib/test/libregrtest/tsan.py Outdated Show resolved Hide resolved
'test_importlib',
'test_io',
'test_logging',
'test_tuple',
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think test_tuple uses threads. I don't think it's worthwhile to run tests that don't use threads, because we won't uncover anything new with TSAN.

Copy link
Member Author

@corona10 corona10 Mar 12, 2024

Choose a reason for hiding this comment

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

I got it; I removed more tests that are not using the thread currently.

@corona10 corona10 closed this Mar 12, 2024
@corona10 corona10 reopened this Mar 12, 2024
@@ -333,6 +334,8 @@ def _create_parser():
help='enable Profile Guided Optimization (PGO) training')
group.add_argument('--pgo-extended', action='store_true',
help='enable extended PGO training (slower training)')
group.add_argument('--tsan', dest='tsan', action='store_true',
help='run a subset of test cases that are proper for the TSAN test')
Copy link
Member Author

Choose a reason for hiding this comment

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

@swtaarrs It will be the more clear?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I completely missed this notification. Looks good to me :)

@erlend-aasland erlend-aasland removed their request for review March 13, 2024 23:33
Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

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

This looks like a good start to me. We can build on it (edit the list of tests and suppressions) in the future.

@corona10 corona10 merged commit ebf29b3 into python:main Mar 15, 2024
40 of 41 checks passed
@corona10 corona10 deleted the gh-112536-tsan-tests branch March 15, 2024 16:07
pitrou pushed a commit to pitrou/cpython that referenced this pull request Mar 17, 2024
… times. (pythongh-116601)

(cherry picked from commit ebf29b3)

Co-authored-by: Donghee Na <donghee.na@python.org>
@bedevere-app
Copy link

bedevere-app bot commented Mar 17, 2024

GH-116929 is a backport of this pull request to the 3.12 branch.

pitrou added a commit that referenced this pull request Mar 18, 2024
gh-116601) (#116929)

(cherry picked from commit ebf29b3)

Co-authored-by: Donghee Na <donghee.na@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants