Reduce memory footprint of Node classes#2790
Conversation
KuechA
requested review from
fwendland,
konradweiss,
lshala,
maximiliankaul,
oxisto and
peckto
as code owners
KuechA
added
the
publish-to-github-packages
oxisto
left a comment
oxisto
left a comment
There was a problem hiding this comment.
So it's good that it seems to lower memory pressure, I suspect it's mainly about the edge collections. Unfortunately they now went from barely manageable code to completely unmanageable code :(
| @Relationship(value = "INVOKES", direction = Relationship.Direction.INCOMING) | ||
| val calledByEdges: Invokes<Function> = | ||
| Invokes<Function>(this, Call::invokeEdges, outgoing = false) | ||
| Invokes<Function>(this, mirroredCollection = { (it as Call).invokeEdges }, outgoing = false) |
There was a problem hiding this comment.
this seems very ... hacky :D can we improve the function/constructor interface somehow to make this more readable? the old style was very readable in contrast.
| import de.fraunhofer.aisec.cpg.persistence.Relationship | ||
| import de.fraunhofer.aisec.cpg.sarif.PhysicalLocation | ||
|
|
||
| interface HasLocals { |
| (if (parent != null) parent.toString() + delimiter else "") + localName | ||
| private var qualifiedFullNameCache: String? = null | ||
|
|
||
| private fun fullName(): String { |
There was a problem hiding this comment.
why is this a function now instead of a property? that changes things on many places in the code unnecessary and might even impact serialization.
Can't you just use val fullName { get() { lazy { .... } } which basically does what you want.
|
|
||
| // For most references, code and name are the same token. If their textual value matches, | ||
| // reuse the name string instance to avoid duplicate string objects. | ||
| if (this is Reference) { |
There was a problem hiding this comment.
Isn't the code already set at this stage? so effectible you are adding more memory pressure here until the GC decides to clean up the original code reference string. I am not sure if this is actually of any benefit
2 participants
We allocate a significant amount of memory in the
Nodeclasses. Partially because we have a lot of attributes (which may not even be used), partially due to inefficient datastructures. This PR implements a couple of things:localsfrom allExpressions and adds it where necessary.EdgeListandEdgeSetimplementations as they often hold a single value but are always aList/Set.Nodes andEdges (e.g.additionalProblems,assumptions).nameascodeforReferences which is the case most of the timeNameclassEdgeListandEdgeSetto speed up the runtimeThis seems to have a positive impact - if my tests were correct this may reduce it from ~10 to ~8.5 GB for a 300k LOC project and also helped to reduce the run-time from ~20min to ~16-18min.