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

Make uop executor objects variable length #106608

Closed
gvanrossum opened this issue Jul 10, 2023 · 6 comments
Closed

Make uop executor objects variable length #106608

gvanrossum opened this issue Jul 10, 2023 · 6 comments
Labels
easy interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@gvanrossum
Copy link
Member

gvanrossum commented Jul 10, 2023

Currently _PyUOpExecutorObject contains a fixed-length (_Py_UOP_MAX_TRACE_LENGTH, 32) array of micro instructions. We should switch to the usual variable-length array (making it a PyVarObject) so that we can make the largest superblock larger without paying the memory overhead cost for the full array.

Linked PRs

@gvanrossum gvanrossum added performance Performance or resource usage interpreter-core (Objects, Python, Grammar, and Parser dirs) easy labels Jul 10, 2023
@gvanrossum
Copy link
Member Author

This should be easy for someone with C experience. There are many examples in the CPython code base that do a similar thing. Basically the struct should be declared to end with _PyUOpInstruction trace[1]; and be given a size field, and various uses of the struct should be updated. I do expect a contributor taking this up to do their own research on how to build CPython, etc.

@dalalvarun
Copy link

I want to give it a try @gvanrossum

@gvanrossum
Copy link
Member Author

@dalalvarun Go for it. Let me know if you get stuck.

@kgdiem
Copy link
Contributor

kgdiem commented Jul 21, 2023

Based on your description above I started off trying to base the implementation of trace off of a PyListObject / PySetObject;

From your description though,PyExecutorArray seemed to fit your description of the implementation better:

Basically the struct should be declared to end with _PyUOpInstruction trace[1]; and be given a size field

typedef struct {
    int size;
    int capacity;
    struct _PyExecutorObject *executors[1];
} _PyExecutorArray;

But this seems to be in conflict with the aim of avoiding having to copy everything into a new, larger array.

My questions:

  1. Is something similar to a PyListObject but without all the casts the aim?
  2. Should there be a new _Py_UOP_MAX_TRACE_LENGTH or should the size of the trace be SSIZE_MAX?

@gvanrossum
Copy link
Member Author

@kgdiem Good questions. What I had in mind is not like PyListObject (which merely has a pointer to a separately malloc'ed array). It's more like a PyTupleObject. The struct you show is closer to what I have in mind, but we don't need a capacity field, and it should be an object. The idea is that the trace is initially written to a temporary buffer (_PyUOpInstruction trace[_Py_UOP_MAX_TRACE_LENGTH] in uop_optimize), and then a _PyUOpExecutorObject is allocated of the right size, into which the trace is copied. The role of _Py_UOP_MAX_TRACE_LENGTH will be reduced to the size of the temporary buffer.

@gvanrossum
Copy link
Member Author

Looks like @rdrf2838 won the race.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage
Projects
None yet
Development

No branches or pull requests

3 participants