Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

small perf improvements in ProtoToGraphNodeMappings and ZipArchive #1800

Merged
merged 2 commits into from
Jan 10, 2025
Merged

small perf improvements in ProtoToGraphNodeMappings and ZipArchive #1800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee64f24
ProtoToGraphNodeMappings: use mutable datastructures internally
maltek Jan 10, 2025
cba2036
ZipArchive: don't return results as linked list
maltek Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 8 additions & 23 deletions ...h/src/main/scala/io/shiftleft/codepropertygraph/cpgloading/ProtoToGraphNodeMappings.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,36 +3,21 @@ package io.shiftleft.codepropertygraph.cpgloading
import flatgraph.*
import io.shiftleft.proto.cpg.Cpg.CpgStruct

import scala.collection.mutable
import scala.jdk.CollectionConverters.*

/** Mutable datastructure to preserve mapping between proto and cpg nodes during ProtoToCpg import.
/** Mutable data structure to preserve mapping between proto and cpg nodes during ProtoToCpg import.
*
* Context: we need to run two passes: 1) add nodes and 2) set node properties and add edges (this is due to
* flatgraph-specific implementation details)
*
* Because of that, we need to remember the mapping from proto node id to gnode. Typically that's just a plain mapping,
* but there's one special case for TYPE nodes: some (parallel) frontends create duplicate TYPE nodes which we need to
* deduplicate...
* Because of that, we need to remember the mapping from proto node id to gnode. Typically, that's just a plain
* mapping. But there's one special case for TYPE nodes: some (parallel) frontends create duplicate TYPE nodes which we
* need to deduplicate...
*/
class ProtoToGraphNodeMappings {
private var protoNodeIdToGNode = Map.empty[Long, DNode]
private var typeFullNameToGNode = Map.empty[String, DNode]

def addAll(other: ProtoToGraphNodeMappings): Unit = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing this since it appears unused, not because there's any problem implementing it with the mutable maps

val intersection1 = this.protoNodeIdToGNode.keySet.intersect(other.protoNodeIdToGNode.keySet)
val intersection2 = this.typeFullNameToGNode.keySet.intersect(other.typeFullNameToGNode.keySet)
assert(
intersection1.isEmpty,
s"unexpected duplicate entries in protoNodeIdToGNode mappings. protoNodeIds: $intersection1"
)
assert(
intersection2.isEmpty,
s"unexpected duplicate entries in typeFullNameToGNode mappings. FullNames: $intersection2"
)

this.protoNodeIdToGNode = this.protoNodeIdToGNode ++ other.protoNodeIdToGNode
this.typeFullNameToGNode = this.typeFullNameToGNode ++ other.typeFullNameToGNode
}
private val protoNodeIdToGNode = mutable.LongMap.empty[DNode]
private val typeFullNameToGNode = mutable.Map.empty[String, DNode]

def add(protoNode: CpgStruct.Node, node: DNode): Unit = {
protoNodeIdToGNode += protoNode.getKey -> node
Expand All @@ -48,7 +33,7 @@ class ProtoToGraphNodeMappings {
}
}

/** This will fail hard if the DiffGraph hasn't been applied yet, which is the assumption for it's use case. In other
/** This will fail hard if the DiffGraph hasn't been applied yet, which is the assumption for its use case. In other
* words, we specifically don't want to invoke `find(protoNode).flatMap(_.storedRef)` here
*/
def findGNode(protoNode: CpgStruct.Node): Option[GNode] =
Expand Down
11 changes: 6 additions & 5 deletions codepropertygraph/src/main/scala/io/shiftleft/codepropertygraph/cpgloading/ZipArchive.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@ package io.shiftleft.codepropertygraph.cpgloading

import java.io.Closeable
import java.nio.file.attribute.BasicFileAttributes
import java.nio.file.{FileSystem, FileSystems, FileVisitResult, Files, Path, Paths, SimpleFileVisitor}
import java.util.{Collection => JCollection}
import java.nio.file.{FileSystem, FileSystems, FileVisitOption, FileVisitResult, Files, Path, Paths, SimpleFileVisitor}
import java.util.Collection as JCollection
import scala.collection.immutable.ArraySeq
import scala.collection.mutable.ArrayBuffer
import scala.jdk.CollectionConverters._
import scala.jdk.CollectionConverters.*

class ZipArchive(inputFile: String) extends Closeable {
private val zipFileSystem: FileSystem = FileSystems.newFileSystem(Paths.get(inputFile), null: ClassLoader)

private def root: Path = zipFileSystem.getRootDirectories.iterator.next

private def walk(rootPath: Path): Seq[Path] = {
val entries = ArrayBuffer[Path]()
val entries = ArraySeq.newBuilder[Path]()
Files.walkFileTree(
rootPath,
new SimpleFileVisitor[Path]() {
Expand All @@ -24,7 +25,7 @@ class ZipArchive(inputFile: String) extends Closeable {
}
}
)
entries.toSeq
entries.result()
}

def entries: Seq[Path] = walk(root)
Expand Down
Loading