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-92123: Pass _elementtree state as parameter #101189

Merged

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Jan 20, 2023

@erlend-aasland
Copy link
Contributor Author

Passing the state around as a parameter is one way to get rid of ET_STATE_GLOBAL. It results in a larger diff (more churn), but IMO it is an ok approach. Alternatively, we can store state as a member in the type contexts, which will result in a smaller diff.

@erlend-aasland
Copy link
Contributor Author

(I will rebase this onto main after #101187 has landed.)

Pass state as arg to create_new_element()
Pass state as arg to deepcopy()
Pass state as arg to expat_set_error()
Pass state as arg to element_setstate_from_attributes()
Pass state as arg to create_elementiter()
Pass state as arg to treebuilder_extend_element_text_or_tail()
Pass state as arg to treebuilder_add_subelement()
Pass state as arg to element_add_subelement()
Pass state as arg to element_setstate_from_Python()
@erlend-aasland erlend-aasland marked this pull request as ready for review January 23, 2023 12:54
@erlend-aasland
Copy link
Contributor Author

cc. @arhadthedev

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Jan 23, 2023

After this, the remaining parts are:

  • use PyModule_GetState where possible (1 small PR)
  • use PyType_GetModuleState where possible (1 slightly larger PR w/clinic changes)
  • use PyType_GetModuleByDef+PyModule_GetState where possible
  • adapt to multi-phase init
  • reapply the pyexpat-c-api-on-the-heap PR

@kumaraditya303
Copy link
Contributor

kumaraditya303 commented Jan 24, 2023

use PyModule_GetState where possible (1 small PR)
use PyType_GetModuleState where possible (1 slightly larger PR w/clinic changes)
use PyType_GetModuleByDef+PyModule_GetState where possible
adapt to multi-phase init

Up to you but I can review of all this in one go, they are relatively small changes.

@kumaraditya303
Copy link
Contributor

Side thought: sizeof(ElementObject) is 56 which rounds off to 64 for alignment so we can add a state pointer and the memory usage will be same.

@erlend-aasland
Copy link
Contributor Author

Side thought: sizeof(ElementObject) is 56 which rounds off to 64 for alignment so we can add a state pointer and the memory usage will be same.

Yes; well observed.

@erlend-aasland erlend-aasland merged commit b2ac396 into python:main Jan 24, 2023
@erlend-aasland erlend-aasland deleted the etree/replace-query-with-param branch January 24, 2023 11:45
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.

3 participants