gh-137838: Move _PyUOpInstruction buffer to PyInterpreterState

[corona10]

• Issue: Make no-opt JIT to pass whole unit test. #137838

[corona10]

@Fidget-Spinner I move _PyUOpInstruction to here becasue of declaration issue. But we may need to move this struct for better place.

[corona10]

I plan to add a new CI for nopt JIT with a separate PR.

[corona10]

And I fully checked that PYTHON_UOPS_OPTIMIZE=0 ./python.exe -m test -j8 is passed on my local.

[Fidget-Spinner]

Looks good, but just come over and ask Mark if he's okay with the struct being in pycore_optimizer.h.

[corona10]

he's okay with the struct being in pycore_optimizer.h.

Now it's currently located in pycore_tstate.h

[Fidget-Spinner]

he's okay with the struct being in pycore_optimizer.h.

Now it's currently located in pycore_tstate.h

Woops yes that's the one I was thinking of. Thanks.

[corona10]

Do you feel comfortable if I define pycore_uop.h?

[Fidget-Spinner]

Do you feel comfortable if I define pycore_uop.h?

That works too.

[corona10]

38 tests failed again:
    test.test_asyncio.test_base_events
    test.test_asyncio.test_buffered_proto
    test.test_asyncio.test_context
    test.test_asyncio.test_eager_task_factory
    test.test_asyncio.test_events
    test.test_asyncio.test_free_threading
    test.test_asyncio.test_futures test.test_asyncio.test_futures2
    test.test_asyncio.test_graph test.test_asyncio.test_locks
    test.test_asyncio.test_pep492
    test.test_asyncio.test_proactor_events
    test.test_asyncio.test_queues test.test_asyncio.test_runners
    test.test_asyncio.test_selector_events
    test.test_asyncio.test_sendfile test.test_asyncio.test_server
    test.test_asyncio.test_sock_lowlevel test.test_asyncio.test_ssl
    test.test_asyncio.test_sslproto test.test_asyncio.test_staggered
    test.test_asyncio.test_streams test.test_asyncio.test_subprocess
    test.test_asyncio.test_taskgroups test.test_asyncio.test_tasks
    test.test_asyncio.test_threads test.test_asyncio.test_timeouts
    test.test_asyncio.test_waitfor
    test.test_asyncio.test_windows_events
    test.test_concurrent_futures.test_interpreter_pool
    test.test_inspect.test_inspect test_asyncgen test_coroutines
    test_external_inspection test_logging test_pdb test_regrtest
    test_unittest

When I move to interpreter state, those tests are failed and could be race condition in some where, need to investgate with Windows machine.

[corona10]

I still need to debug this in detail, but the buffer causes problems when it’s shared at the interpreter level under an asyncio workload. A per-thread approach also seems safe for the GIL build.
Before this PR, it was safe because it was allocated in stack-wise while the optimizer is needed.

[corona10]

(3460.15a4): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
_asyncio_d!add_tasks_llist+0x20:
00007ffe`eb88fe50 488b00          mov     rax,qword ptr [rax] ds:00000000`00000000=????????????????

[corona10]

Okay it turn out that it is not related to this change but looks like trigger other bug in asyncio. Let me test..

[corona10]

@brandtbucher @Fidget-Spinner Fixed! Ready to be merged!

[corona10]

The issue was that I declared this field only for the TIER2 build.
We might need to adjust it properly if we want to exclude non-JIT builds.
But since the field is only allocated when the TIER2 build is enabled, I think it’s fine to leave it as is.

[markshannon]

Thanks for doing this.

I have a few comments:

1. If we define UOP_MAX_TRACE_LENGTH in terms of the size of the buffer, we won't leak the optimizer details to the thread state.
   i.e.

#define UOP_MAX_TRACE_LENGTH JIT_TRACE_BUFFER_SIZE/sizeof(_PyUOpInstruction)

2. It might be better to use _PyObject_VirtualAlloc so that we can over allocate without wasting physical memory.

3. Can we add asserts that the optimizer doesn't re-enter so that we don't overwrite the buffer while we are using it?

[corona10]

@brandtbucher _PyObject_VirtualFree doesn’t return a value, and the current CPython codebase doesn’t check whether it succeeds or not

[corona10]

@markshannon Hopefully you will like the current change :) I 've applied most of the changes you requested!

[markshannon]

Looks good now.
Thanks