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

Refactor: gc flags and friends #16153

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

withinboredom
Copy link
Contributor

While trying to wrap my head around GC and GC_FLAGS, I found the current setup a bit confusing and brittle. For example, if you want to add a new flag, you'll end up with a very subtle memory leak unless you know to update GC_ADDRESS and how that affects the GC. Diagnosing this memory leak is not straightforward either.

Thus I set out to refactor the macros here to be a bit easier to tune and comprehend; mostly to increase my own understanding of the code. I hope that other people might find it valuable as well, so I also added a full layout explaining more about what is in each part as a hint (depending on why you find yourself there).

Zend/zend_types.h Outdated Show resolved Hide resolved
Copy link
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't make things more clear.

Actually, when you have to "read" some hex numbers printed during debug session, 0x000003F0 mask is more useful than (((1U << GC_FLAGS_BITS) - 1) << (GC_FLAGS_SHIFT + GC_TYPE_BITS)).

@withinboredom
Copy link
Contributor Author

Maybe "understandable" is the wrong word? I think the word I am going for is "approachable." Currently, in the docs and existing code, the relationships and how to make changes here are a "dark art" that you either learn through PRs, are taught, or work out through painstaking debugging.

This change seeks to make these values a product of their underlying parameters, both documenting how they are calculated as well as their intrinsic dependence on each other. Dumping the actual hex values should be a relatively easy exercise for any developer, should they need to be known.

Could this be solved through documentation? Almost certainly. However, documentation rarely survives the first contact with a refactor, and I don't have the ability to update any documentation anyway. So this PR is as close as I can get to documenting how this works and why it is the way it is.

@dstogov
Copy link
Member

dstogov commented Oct 2, 2024

One of the best ways is readable "graphical" comments. For example see:

* HashTable Data Layout
and
* Stack Frame Layout (the whole stack frame is allocated at once)

The same is possible for bit-layout. For example:
https://github.com/LuaJIT/LuaJIT/blob/2240d84464cc3dcb22fd976f1db162b36b5b52d5/src/lj_obj.h#L226
https://github.com/LuaJIT/LuaJIT/blob/2240d84464cc3dcb22fd976f1db162b36b5b52d5/src/lj_bc.h#L12
https://github.com/LuaJIT/LuaJIT/blob/2240d84464cc3dcb22fd976f1db162b36b5b52d5/src/lj_ir.h#L541

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