Skip to content

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 force-pushed the etree/replace-query-with-param branch from b853fba to fbd465a Compare January 23, 2023 12:37
@erlend-aasland erlend-aasland force-pushed the etree/replace-query-with-param branch from fbd465a to 0195c35 Compare January 23, 2023 12:41
@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