Skip to content

Use 0 for disabling automatic GC #421

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

Closed
wants to merge 2 commits into from
Closed

Use 0 for disabling automatic GC #421

wants to merge 2 commits into from

Conversation

saghul
Copy link
Contributor

@saghul saghul commented Jun 4, 2024

Similarly to other limits such as memory or stack limit, 0 means unlimited.

Similarly to other limits such as memory or stack limit, 0 means
unlimited.
@saghul saghul requested a review from chqrlie June 4, 2024 15:37
@KaruroChori
Copy link
Contributor

KaruroChori commented Jun 4, 2024

I am not sure I agree with the implied semantic of this change.
Right now, setting it to zero has the meaning of "please run it next time there is an allocation".
Setting it to -1 is basically +infy in unsigned land and so "you must overflow infinity before I will allow you to run".

To me it is much more sensible than an arbitrary 0 is disabled. It is called threshold, I think it ought to behave as such.

Aside from the fact it is a pretty breaking change.

@chqrlie
Copy link
Collaborator

chqrlie commented Jun 4, 2024

I am not sure I agree with the implied semantic of this change. Right now, setting it to zero has the meaning of "please run it next time there is an allocation". Setting it to -1 is basically +infy in unsigned land and so "you must overflow infinity before I will allow you to run".

Technically, the GC is only run upon an object allocation, so adding a zillion properties to an existing object does not trigger the GC, which might be a shortcoming. Since we use reference counting, the GC is only useful if webs of mutually referring objects or self referring objects have been cut from the graph of objects accessible from the global object. We should probably try and come up with a more sensible way to assess the presence of such occurrences than just counting object allocations.

To me it is much more sensible than an arbitrary 0 is disabled. It is called threshold, I think it ought to behave as such.

Aside from the fact it is a pretty breaking change.

Yes, I agree this might not be a good change.

@saghul
Copy link
Contributor Author

saghul commented Jun 4, 2024

To me it is much more sensible than an arbitrary 0 is disabled. It is called threshold, I think it ought to behave as such.
Aside from the fact it is a pretty breaking change.

Yes, I agree this might not be a good change.

It aligns the API with the other limiting APIs we have, like the memory and stack limit.

@saghul
Copy link
Contributor Author

saghul commented Jun 6, 2024

I split the second commit here: #424

@saghul saghul closed this Jan 13, 2025
@saghul saghul deleted the gc-threshold branch March 8, 2025 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants