Simplify the GC.gc interface #34303
Simplify the GC.gc interface #34303
Conversation
On the latest Julia this API is equivalent to the explicit use. Ref JuliaLang/julia#34303
(cherry picked from commit b0ed147)
|
|
||
| !!! warning | ||
| Excessive use will likely lead to poor performance. | ||
| """ | ||
| gc(full::Bool=true) = ccall(:jl_gc_collect, Cvoid, (Cint,), full) | ||
| gc(collection::CollectionType) = ccall(:jl_gc_collect, Cvoid, (Cint,), collection) | ||
| gc() = ccall(:jl_gc_collect, Cvoid, (Cint,), 0) |
There was a problem hiding this comment.
Sorry, read this too quickly since it was supposed to be not a breaking change. This was a breaking change (as in CI has been broken for a week now because of this PR):
| gc() = ccall(:jl_gc_collect, Cvoid, (Cint,), 0) | |
| gc() = ccall(:jl_gc_collect, Cvoid, (Cint,), 1) |
Please, please, please be careful about merging PRs that fail on CI (as this one did).
There was a problem hiding this comment.
Even if they are not wrapped in a type or user-facing, it might be good for this code to use locally-defined constants to make the code easier to read. My understanding is that these are the values:
const GC_AUTO = 0
const GC_FULL = 1
const GC_INCR = 2Is that correct? If so, it would be good to define these constants and then use them in the method definitions. I guess the issue is that the default previously was GC_FULL rather than GC_AUTO?
There was a problem hiding this comment.
Crap, sorry for that, I had been too accustomed to win32 failing recently...
Even if they are not wrapped in a type or user-facing, it might be good for this code to use locally-defined constants to make the code easier to read.
Agreed, I'll see about improving that next week.
There was a problem hiding this comment.
Crap, sorry for that, I had been too accustomed to win32 failing recently...
💯 I wouldn't blame you here. CI needs to be fixed. The situation is atrocious.
(cherry picked from commit b0ed147)
Implements #33448 (comment). Previous API is only part of 1.4, so this is not breaking (yet).