Skip to content

Conversation

@gvanrossum
Copy link
Member

@gvanrossumgvanrossum commented Jul 1, 2023

A bit of an omnibus PR, alas (let me know if you want it split up). This does roughly the following (though not in that order):

- Longer traces - Explain reason for ending a trace - Support STORE_FAST_LOAD_FAST, STORE_FAST_STORE_FAST - Add deps on pycore_uops.h
This gives us LOAD_FAST_CHECK, LOAD_CLOSURE, and DELETE_FAST.
This doesn't work right yet, alas. If an error occurs in the first uop, the lineno is -1.
The trick is that EXIT_TRACE must decrement prev_instr.
@gvanrossum
Copy link
MemberAuthor

While we're cleaning up uops, maybe we should change "trace" to "superblock"? There are already several other things using the name "trace", e.g. sys.settrace(), tracemalloc, lltrace.

@gvanrossum
Copy link
MemberAuthor

This (with uops on by default) benchmarks 2% slower than main (with uops off by default).

I will now revert the final (temporary) commit that turns on uops by default and mark it as ready for review.

(I just did that to be able to benchmark it and run the tests.) This reverts commit 5b85a49.
@gvanrossumgvanrossum marked this pull request as ready for review July 2, 2023 03:16
@gvanrossum
Copy link
MemberAuthor

(Also, the tests that fail with uops on by default are largely expected. There's one refleak -- maybe I should ask Kumar to review it again. :-)

@gvanrossum
Copy link
MemberAuthor

Maybe the leak is the optimizer created and installed initially.

Copy link
Member

@brandtbucherbrandtbucher left a comment

Choose a reason for hiding this comment

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

The code itself looks good, haven't tried running it myself or anything. Just one note:

return0;

#undef ADD_TO_TRACE
#undef ADD_TO_TRACE_BASIC
Copy link
Member

Choose a reason for hiding this comment

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

I don't see where this is defined...

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Turd from the last cleanup. I'll get rid of it, then land it.

Use it to `#undef DPRINTF` instead (which also depends on a variable in this scope).
@gvanrossumgvanrossum enabled auto-merge (squash) July 3, 2023 19:29
@gvanrossumgvanrossum merged commit 2028a4f into python:mainJul 3, 2023
@bedevere-bot
Copy link

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

Hi! The buildbot ARM64 macOS 3.x has failed when building commit 2028a4f.

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/725/builds/4943) and take a look at the build logs.
  4. Check if the failure is related to this commit (2028a4f) 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/725/builds/4943

Failed tests:

  • test_unittest

Failed subtests:

  • test_wait_until_any_call_positional - test.test_unittest.testmock.testthreadingmock.TestThreadingMock.test_wait_until_any_call_positional

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

== Tests result: FAILURE then FAILURE ==

420 tests OK.

10 slowest tests:

  • test_signal: 4 min 13 sec
  • test_concurrent_futures: 2 min 49 sec
  • test_multiprocessing_spawn: 2 min 19 sec
  • test_multiprocessing_forkserver: 1 min 41 sec
  • test_math: 1 min 38 sec
  • test_logging: 1 min 3 sec
  • test_capi: 54.7 sec
  • test_unparse: 49.4 sec
  • test_tokenize: 45.2 sec
  • test_ssl: 43.1 sec

1 test failed:
test_unittest

22 tests skipped:
test.test_asyncio.test_windows_events
test.test_asyncio.test_windows_utils test_devpoll test_epoll
test_gdb test_idle test_ioctl test_launcher
test_multiprocessing_fork test_perf_profiler test_perfmaps
test_startfile test_tcl test_tkinter test_ttk test_ttk_textonly
test_turtle test_winconsoleio test_winreg test_winsound test_wmi
test_zipfile64

1 re-run test:
test_unittest

Total duration: 10 min 6 sec

Click to see traceback logs
Traceback (most recent call last): File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/test_unittest/testmock/testthreadingmock.py", line 173, in test_wait_until_any_call_positionalself.assertNotIn(call(2), something.method_1.mock_calls) AssertionError: call(2) unexpectedly found in [call(2), call(1)] Traceback (most recent call last): File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/test_unittest/testmock/testthreadingmock.py", line 172, in test_wait_until_any_call_positional something.method_1.assert_called_with(1) File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/unittest/mock.py", line 979, in assert_called_withraiseAssertionError(_error_message()) from cause AssertionError: expected call not found.Expected: method_1(1)Actual: method_1(2)

@gvanrossumgvanrossum deleted the tweak-uops branch July 5, 2023 15:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@gvanrossum@bedevere-bot@brandtbucher