GeezerVison

@vstinner @shihai1991 @corona10 please review

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

The patch looks like it was a lot of work. Thanks for investing your time to port _elementtree.

I'm a bit worried that the module state getter is going to affect performance negatively. The code has to use the slow _PyType_GetModuleByDef() API. In my patch for the _ssl module I added a module state pointer to instance structs. For example you could add state to typedef struct ElementObject and then reference the state with self->state. This would reduce the amount of get state calls and simplify the code.

What do you think? Is this a safe apprach?

@tiran I'm not sure if its safe but I understand it in principle. I don't know exactly the lifetime of instance structs when pickling. @vstinner thoughts on that approach?

Is there a benchmark I could try to measure the performance impact? _PyType_GetModuleByDef seems to only be slow if there is significant subclassing so I could try to whip up an example that tries to overdo that.

This PR introduces a ref. leak (looks similar to the ref. leak I had in PR #23428):

bash$ ./python.exe -m test -F -r -j1 -R 3:5 test_xml_etree_c
Using random seed 1742818
0:00:00 load avg: 1.54 Run tests in parallel using 1 child processes
0:00:14 load avg: 1.50 [  1/1] test_xml_etree_c failed
beginning 8 repetitions
12345678
........
test_xml_etree_c leaked [147, 147, 147, 147, 147] references, sum=735
test_xml_etree_c leaked [59, 59, 59, 59, 59] memory blocks, sum=295

== Tests result: FAILURE ==

1 test failed:
    test_xml_etree_c

Total duration: 14.8 sec
Tests result: FAILURE

@erlend-aasland oof sorry, I didn't realize you were working on this module too. Any tips to find the cause of the ref leak or which reference is leaking? I'm not too familiar with the diagnostics & telemetry available.

oof sorry, I didn't realize you were working on this module too.

No problem, you're fine! Feel free to take a look at my PR. I haven't been able to devote any free time to this the last month or so, anyway.

Any tips to find the cause of the ref leak or which reference is leaking? I'm not too familiar with the diagnostics & telemetry available.

Me neither. It kinda stopped when I encountered the ref. leak. I use python.exe -m test -F -r -j1 -R 3:5 test_<mod> to check for reference/memory leaks. I've tried playing with some of the -X options as well, but I haven't found a good way to debug ref leaks yet. I'll keep an eye on this PR to see what you find out :)

This PR is stale because it has been open for 30 days with no activity.

https://bugs.python.org/issue1635741 is closed. What is that status of this PR?

The change is still relevant, but should use a new issue number.

Moreover, the SC asked to put the conversion of static types to heap types on hold. @encukou and @erlend-aasland wrote https://peps.python.org/pep-0687/ which may unblock the situation but it's still a draft.

FYI, PEP-687 was just accepted.

Superseded by gh-101285