Fixes #16513: JVM GC cannot clean objects in scope in a for { } yield {} even if they are not referenced anymore #2709
Conversation
… {} even if they are not referenced anymore
| _ = PolicyLogger.debug(s"Node configuration written in ${timeWriteNodeConfig} ms, start to update expected reports.") | ||
| (updatedNodesId, updatedNodeInfo, expectedReports, allErrors, errorNodes, timeFetchAll, timeHistorize, timeRuleVal, timeBuildConfig, timeWriteNodeConfig, timeSetExpectedReport) <- for { | ||
| (updatedNodeConfigIds, allNodeConfigurations, allNodeConfigsInfos, updatedNodesId, updatedNodeInfo, allLicenses, globalPolicyMode, allNodeModes, allErrors, errorNodes, timeFetchAll, timeHistorize, timeRuleVal, timeBuildConfig) <- for { | ||
| (activeNodeIds, ruleVals, nodeContexts, allNodeModes, scriptEngineEnabled, globalPolicyMode, nodeConfigCaches, allLicenses, timeFetchAll, timeHistorize, timeRuleVal) <- for { |
There was a problem hiding this comment.
The scoping is quickly becoming hard to follow I believe. Wouldn't it be clearer to factor out the inner-most for in dedicated support function ? (I'm not sure, because we already have tons of them, and we will likely loose the general understanding of the policy generation workflow). What's your opinion on that?
There was a problem hiding this comment.
Or perhaps build dedicated case objects that only handle the relevant part and are passed around ?
(well, I fear that it will make refactoring harder, since we will have a hard time to know when we can remove an object from such a structure, so perhaps a bad idea actually)
There was a problem hiding this comment.
we already have a lot of back and forth with def, making it hard to follow. Plus, there are timestamp for loging within this, so adding them in a def is nightmarish
There was a problem hiding this comment.
I wish there were a way to make it better (an annotation that the compiler could use to know to release quickly some variable)
|
This PR is not mergeable to upper versions. |
|
OK, merging this PR |
https://issues.rudder.io/issues/16513