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

Isolate the _decimal extension module #106078

Closed
14 tasks done
Tracked by #103092
CharlieZhao95 opened this issue Jun 25, 2023 · 8 comments
Closed
14 tasks done
Tracked by #103092

Isolate the _decimal extension module #106078

CharlieZhao95 opened this issue Jun 25, 2023 · 8 comments
Labels
extension-modules C modules in the Modules dir topic-subinterpreters type-feature A feature request or enhancement

Comments

@CharlieZhao95
Copy link
Contributor

CharlieZhao95 commented Jun 25, 2023

This issue is used to track the PRs split from gh-103092. Isolate the _decimal extension module by:

  • Establish a global module state and convert static types to heap types
  • Move other global static variables to the global module state(possibly multiple PRs)
    • DecimalException
    • basic_context_template
    • tls_context_key
    • cached_context
    • current_context_var
    • default_context_template
    • extended_context_template
    • round_map
    • Rational
    • SignalTuple
    • External C-API functions
  • Convert the global module state to true module state
    • signal_map
    • cond_map
    • convert global state to module state

See original issue Isolate Stdlib Extension Modules and PEP-687 for details.

Linked PRs

@CharlieZhao95 CharlieZhao95 added the type-feature A feature request or enhancement label Jun 25, 2023
erlend-aasland added a commit that referenced this issue Jun 29, 2023
- Establish global state struct
- Convert static types to heap types and add them to global state:
    * PyDecContextManager_Type
    * PyDecContext_Type
    * PyDecSignalDictMixin_Type
    * PyDec_Type
- Add to global state:
    * PyDecSignalDict_Type
    * DecimalTuple

Co-authored-by: Kumar Aditya <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
@erlend-aasland
Copy link
Contributor

What is your next step, @CharlieZhao95?

@CharlieZhao95
Copy link
Contributor Author

What is your next step, @CharlieZhao95?

I think next we can move the remaining static global PyObject to global_state:

  • DecimalException
  • basic_context_template
  • current_context_var
  • default_context_template
  • extended_context_template
  • round_map
  • Rational
  • SignalTuple
  • External C-API functions

This shouldn't be too complicated. The only question is whether we should move these external-API functions, which simply hold function pointers used by numeric types, such as nb_multiply, nb_floor_divide and so on. They will not change during the lifecycle of the program.

@erlend-aasland
Copy link
Contributor

Pick the low-hanging fruit first. It is better with multiple small PRs than one big.

@CharlieZhao95
Copy link
Contributor Author

Pick the low-hanging fruit first. It is better with multiple small PRs than one big.

Okay! let's go step by step. I will put the objects listed above into multiple small PRs.

carljm added a commit to carljm/cpython that referenced this issue Jul 3, 2023
* main: (167 commits)
  pythongh-91053: make func watcher tests resilient to other func watchers (python#106286)
  pythongh-104050: Add more type hints to Argument Clinic DSLParser() (python#106354)
  pythongh-106359: Fix corner case bugs in Argument Clinic converter parser (python#106361)
  pythongh-104146: Remove unused attr 'parameter_indent' from clinic.DLParser (python#106358)
  pythongh-106320: Remove private _PyErr C API functions (python#106356)
  pythongh-104050: Annotate Argument Clinic DSLParser attributes (python#106357)
  pythongh-106320: Create pycore_modsupport.h header file (python#106355)
  pythongh-106320: Move _PyUnicodeWriter to the internal C API (python#106342)
  pythongh-61215: New mock to wait for multi-threaded events to happen (python#16094)
  Document PYTHONSAFEPATH along side -P (python#106122)
  Replace the esoteric term 'datum' when describing dict comprehensions (python#106119)
  pythongh-104050: Add more type hints to Argument Clinic DSLParser() (python#106343)
  pythongh-106320: _testcapi avoids private _PyUnicode_EqualToASCIIString() (python#106341)
  pythongh-106320: Add pycore_complexobject.h header file (python#106339)
  pythongh-106078: Move DecimalException to _decimal state (python#106301)
  pythongh-106320: Use _PyInterpreterState_GET() (python#106336)
  pythongh-106320: Remove private _PyInterpreterState functions (python#106335)
  pythongh-104922: Doc: add note about PY_SSIZE_T_CLEAN (python#106314)
  pythongh-106217: Truncate the issue body size of `new-bugs-announce-notifier` (python#106329)
  pythongh-104922: remove PY_SSIZE_T_CLEAN (python#106315)
  ...
kumaraditya303 pushed a commit that referenced this issue Jul 8, 2023
… module global state (#106395)

Co-authored-by: Erlend E. Aasland <[email protected]>
@CharlieZhao95
Copy link
Contributor Author

The second stage has been completed. All documented static variables are moved to the global state. Thanks for review! @erlend-aasland @kumaraditya303

Based on the experience of the original PR, we should also move the signal_map into the module state before converting the global state. This array of structures contains potentially global variables(ex).

typedef struct {
    const char *name;   /* condition or signal name */
    const char *fqname; /* fully qualified name */
    uint32_t flag;      /* libmpdec flag */
    PyObject *ex;       /* corresponding exception */
} DecCondMap;

static DecCondMap signal_map[] = {
    ...
}

IMO, the next stage should be:

  1. Move signal_map to global state
  2. Convert the global module state to true module state

How about opening a draft PR (convert the global module) first? This will allow us to see failed test cases raised by signal_map more clearly. Or do I just start moving signal_map into global state?

@erlend-aasland
Copy link
Contributor

How about opening a draft PR (convert the global module) first? This will allow us to see failed test cases raised by signal_map more clearly. Or do I just start moving signal_map into global state?

FTR, I often open PRs on my own fork to check CI issues, so that I won't pollute the CPython PR list too much.

Also: I'll be away for the rest of summer, so my response time will probably be slower than usual, just so you know :)

@CharlieZhao95

This comment was marked as outdated.

kumaraditya303 pushed a commit that referenced this issue Jul 20, 2023
* move signal_map to global_state

* move cond_map to global_state
@erlend-aasland
Copy link
Contributor

Great work, @CharlieZhao95 and @kumaraditya303!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir topic-subinterpreters type-feature A feature request or enhancement
Projects
Status: Done
Development

No branches or pull requests

3 participants