hotfix: suppress GC in heartbeat #3623
Conversation
Co-authored-by: Gabor Greif <gabor@dfinity.org>
| @@ -3894,7 +3894,11 @@ module IC = struct | |||
| let fi = E.add_fun env "canister_heartbeat" | |||
| (Func.of_body env [] [] (fun env -> | |||
| G.i (Call (nr (E.built_in env "heartbeat_exp"))) ^^ | |||
| GC.collect_garbage env)) | |||
| (* TODO(3622) | |||
There was a problem hiding this comment.
I saw this while implementing system func timer, and I remember considering it a bit expensive/dangerous. But with DTS not covering this, yuck!
There was a problem hiding this comment.
Yeah, I remember you suggesting it but thought better not to mess with the GC. This just a temp hack. Let's see what Luc thinks.
|
Looks good. I also checked it with an example (that creates garbage in the heartbeat). The question is whether this extra GC schedule call is needed at all in the future even if DTS is available for the heartbeat call (since the async function contains GC scheduling anyway). |
My initial intention was to hopefully remove the extra async hop in future, so the heartbeat code runs during the heartbeat method. The fact that it runs with a delay is an accident to the async/await implementation (that async*/await* will optimize). If we do that, we'll need to do the GC I think, otherwise an inactive canister will just leak memory due to heartbeats. |
Mainnet doesn't yet support DTS for
canister_heartbeatyet, so a long GC may fail to complete when it would in an ordinary message supporting DTS.This PR suppresses the GC call, leaving it to the already scheduled async message executing Motoko
systemfunctionheartbeat, which will run with DTS enabled, avoiding the issue (and a potential space leak).#3622 tracks the need to revert this hack once IC heartbeats support DTS.