Skip to content

Commit

Permalink
Fixes #12702: Method copyResourceFile is quite inefficient
Browse files Browse the repository at this point in the history
  • Loading branch information
ncharles committed Jun 27, 2018
1 parent 426bbba commit aa61ede
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ class Cf3PromisesFileWriterServiceImpl(
configAndPaths <- calculatePathsForNodeConfigurations(interestingNodeConfigs, rootNodeId, allNodeConfigs, newPostfix, backupPostfix)
pathsInfo = configAndPaths.map { _.paths }
templates <- readTemplateFromFileSystem(techniqueIds)
resources <- readResourcesFromFileSystem(techniqueIds)

readTemplateTime2 = DateTime.now.getMillis
readTemplateDur = readTemplateTime2 - readTemplateTime1
Expand All @@ -307,7 +308,7 @@ class Cf3PromisesFileWriterServiceImpl(

promiseWritten <- parrallelSequence(preparedPromises) { prepared =>
(for {
_ <- writePromises(prepared.paths, prepared.preparedTechniques)
_ <- writePromises(prepared.paths, prepared.preparedTechniques, resources)
_ <- writeExpectedReportsCsv(prepared.paths, prepared.expectedReportsCsv, GENEREATED_CSV_FILENAME)
_ <- writeSystemVarJson(prepared.paths, prepared.systemVariables)
} yield {
Expand Down Expand Up @@ -491,6 +492,45 @@ class Cf3PromisesFileWriterServiceImpl(
res
}

/*
* We are returning a map where keys are (TechniqueResourceId, AgentType) because
* for a given resource IDs, you can have different out path for different agent.
*/
private[this] def readResourcesFromFileSystem(
techniqueIds: Set[TechniqueId]
)(implicit scheduler: Scheduler, semaphore: TaskSemaphore): Box[Map[TechniqueResourceId, TechniqueResourceCopyInfo]] = {

val staticResourceToRead = for {
technique <- techniqueRepository.getByIds(techniqueIds.toSeq)
staticResource <- technique.files.map(t => (t.id, t.outPath))
} yield {
staticResource
}

val now = System.currentTimeMillis()

val res = (parrallelSequence(staticResourceToRead) { case (templateId, templateOutPath) =>
for {
copyInfo <- techniqueRepository.getFileContent(templateId) { optInputStream =>
optInputStream match {
case None =>
Failure(s"Error when trying to open template '${templateId.toString}'. Check that the file exists with a ${TechniqueTemplate.templateExtension} extension and is correctly commited in Git, or that the metadata for the technique are corrects.")
case Some(inputStream) =>
logger.trace(s"Loading template: ${templateId}")
//string template does not allows "." in path name, so we are force to use a templateGroup by polity template (versions have . in them)
val content = IOUtils.toString(inputStream, Codec.UTF8.charSet)
Full(TechniqueResourceCopyInfo(templateId, templateOutPath, content))
}
}
} yield {
(copyInfo.id, copyInfo)
}
}).map( _.toMap)

logger.debug(s"${staticResourceToRead.size} promises resources read in ${System.currentTimeMillis-now} ms")
res
}

private[this] def writeExpectedReportsCsv(paths: NodePromisesPaths, csv: ExpectedReportsCsv, csvFilename: String): Box[String] = {
val path = new File(paths.newFolder, csvFilename)
for {
Expand Down Expand Up @@ -545,8 +585,9 @@ class Cf3PromisesFileWriterServiceImpl(


private[this] def writePromises(
paths : NodePromisesPaths
paths : NodePromisesPaths
, preparedTechniques: Seq[PreparedTechnique]
, resources : Map[TechniqueResourceId, TechniqueResourceCopyInfo]
) : Box[NodePromisesPaths] = {
// write the promises of the current machine and set correct permission
for {
Expand All @@ -556,7 +597,7 @@ class Cf3PromisesFileWriterServiceImpl(
writePromisesFiles(template, preparedTechnique.environmentVariables , paths.newFolder)
}
files <- sequence(preparedTechnique.filesToCopy.toSeq) { file =>
copyResourceFile(file, paths.newFolder)
copyResourceFile(file, paths.newFolder, resources)
}
} yield {
"OK"
Expand Down Expand Up @@ -657,21 +698,22 @@ class Cf3PromisesFileWriterServiceImpl(
/**
* Copy a resource file from a technique to the node promises directory
*/
private[this] def copyResourceFile(file: TechniqueFile, rulePath: String): Box[String] = {
private[this] def copyResourceFile(
file: TechniqueFile
, rulePath: String
, resources : Map[TechniqueResourceId, TechniqueResourceCopyInfo]
): Box[String] = {
val destination = new File(rulePath+"/"+file.outPath)

techniqueRepository.getFileContent(file.id) { optStream =>
optStream match {
case None => Failure(s"Can not open the technique resource file ${file.id} for reading")
case Some(s) =>
try {
FileUtils.copyInputStreamToFile(s, destination)
Full(destination.getAbsolutePath)
} catch {
case ex: Exception => Failure(s"Error when copying technique resoure file '${file.id}' to '${destination.getAbsolutePath}')", Full(ex), Empty)
}
}

resources.get(file.id) match {
case None => Failure(s"Can not open the technique resource file ${file.id} for reading")
case Some(s) =>
try {
FileUtils.writeStringToFile(destination, s.content, Codec.UTF8.charSet)
Full(destination.getAbsolutePath)
} catch {
case ex: Exception => Failure(s"Error when copying technique resoure file '${file.id}' to '${destination.getAbsolutePath}')", Full(ex), Empty)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,5 +120,14 @@ case class TechniqueTemplateCopyInfo(
, destination: String
, content : String //template content as a file
) extends HashcodeCaching {
override def toString() = s"Promise template ${id.name}; destination ${destination}"
override def toString() = s"Technique template ${id.name}; destination ${destination}"
}


case class TechniqueResourceCopyInfo(
id : TechniqueResourceId
, destination: String
, content : String //template resource as a file
) extends HashcodeCaching {
override def toString() = s"Technique resource ${id.name}; destination ${destination}"
}

0 comments on commit aa61ede

Please sign in to comment.