JuliaLang / AllocCheck.jl

AllocCheck
Other
215 stars 8 forks source link

Mark GC API functions as non-allocating #26

Closed topolarity closed 10 months ago

topolarity commented 10 months ago

These might enable/disable garbage collection, but they don't themselves trigger a safepoint or allocate objects.

Open Question: What should we do with GC.safepoint() and GC.gc()? They are technically not allocating, but many users may want to prohibit them anyway.

baggepinnen commented 10 months ago

If one of the functions GC.safepoint() and GC.gc() has been explicitly called, I take that as a rather explicit opt in to what they do. However, the person calling check_allocs may not be the same as the library writer who called the GC.x functions, so I'm a bit torn as to what the best way to handle them is.

Would a user option ignore_explicit_gc, similar to ignore_throw, be an option?

topolarity commented 10 months ago

Would a user option ignore_explicit_gc, similar to ignore_throw, be an option?

It's an option yeah, but I have some ambivalent feelings about it

Usually, I think of each flag as a bit of complexity that will make it harder for users to answer the question "What does this tell me about my function?" so I'd like to avoid them as much as possible.

topolarity commented 10 months ago

@gbaraldi pointed out to me that GC.safepoint() doesn't actually trigger a GC. It just provides a place for execution on this thread to pause, in case GC is waiting to start from somewhere else.

So I guess the only question is whether we want to count GC.gc() as an allocation or not.

baggepinnen commented 10 months ago

I think of each flag as a bit of complexity that will make it harder for users to answer the question "What does this tell me about my function?"

If we mark it as an allocation when it actually isn't, wouldn't that be confusing and require explanation in the docs as well?

In the end, I think any option we go with here would be workable, if it's left as an option the user picks the option that best suits them, if it's marked as an allocation, the user could instead check the returned allocation sites and {care about / ignore} any allocation associated with GC.gc().

topolarity commented 10 months ago

Let's merge this for now, and if we want to add a check for GC.gc() calls we can circle back later