From b21249d55c33aa00705211a9d6689b1356828bba Mon Sep 17 00:00:00 2001 From: Clint Valentine Date: Mon, 7 Oct 2019 16:30:45 -0700 Subject: [PATCH 1/2] PicardTask to use the Intel deflater/inflater only if safe, by default --- .../dagr/core/cmdline/DagrCoreMain.scala | 42 +++++++++++++++++++ .../scala/dagr/tasks/picard/PicardTask.scala | 10 ++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/dagr/core/cmdline/DagrCoreMain.scala b/core/src/main/scala/dagr/core/cmdline/DagrCoreMain.scala index 7426dda8..38789b93 100644 --- a/core/src/main/scala/dagr/core/cmdline/DagrCoreMain.scala +++ b/core/src/main/scala/dagr/core/cmdline/DagrCoreMain.scala @@ -43,6 +43,48 @@ import dagr.core.tasksystem.Pipeline import scala.collection.mutable.ListBuffer import scala.util.{Failure, Success} + +/** A little class so that we don't have to rely on SystemUtils from org.apache.commons:commons-lang3. + * + * This implementation is duplicated from `com.fulcrumgenomics.cmdline.SystemUtils`. + */ +private[dagr] object SystemUtils { + /** The OS name prefixes for Linux */ + private val LinuxNamePrefixes: Seq[String] = Seq("Linux", "LINUX") + /** The OS name prefixes for Mac */ + private val MacNamePrefixes: Seq[String] = Seq("Mac") + /** The current OS name. */ + private val OsName: Option[String] = getSystemProperty("os.name") + /** The current OS architecture */ + private val OsArch: Option[String] = getSystemProperty("os.arch") + /** The current OS version */ + private val OsVersion: Option[String] = getSystemProperty("os.version") + + /** Gets a system property. Returns None if not found or not allowed to look at. */ + private def getSystemProperty(property: String): Option[String] = { + try { + Option(System.getProperty(property)) + } catch { case _: SecurityException => None } // not allowed to look at this property + } + + /** True if this OS is Linux, false otherwise. */ + private val IsOsLinux: Boolean = LinuxNamePrefixes.exists(prefix => OsName.exists(_.startsWith(prefix))) + /** True if this OS is Mac, false otherwise. */ + private val IsOsMac: Boolean = MacNamePrefixes.exists(prefix => OsName.exists(_.startsWith(prefix))) + /** Returns true if the architecture is the given name, false otherwise. */ + private def IsOsArch(name: String): Boolean = OsArch.contains(name) + /** Returns true if the operating system version starts with the given version string, false otherwise. */ + private def IsOsVersion(prefix: String): Boolean = OsVersion.exists(_.startsWith(prefix)) + + /** True if the current system supports the Intel Inflater and Deflater, false otherwise. */ + val IntelCompressionLibrarySupported: Boolean = { + if (!SystemUtils.IsOsLinux && !SystemUtils.IsOsMac) false + else if (SystemUtils.IsOsArch("ppc64le")) false + else if (SystemUtils.IsOsMac && SystemUtils.IsOsVersion("10.14.")) false // FIXME: https://github.com/Intel-HLS/GKL/issues/101 + else true + } +} + object DagrCoreMain extends Configuration { /** The packages we wish to include in our command line **/ protected def getPackageList: List[String] = { diff --git a/tasks/src/main/scala/dagr/tasks/picard/PicardTask.scala b/tasks/src/main/scala/dagr/tasks/picard/PicardTask.scala index dcd0dce6..9daa039c 100644 --- a/tasks/src/main/scala/dagr/tasks/picard/PicardTask.scala +++ b/tasks/src/main/scala/dagr/tasks/picard/PicardTask.scala @@ -25,6 +25,7 @@ package dagr.tasks.picard import java.nio.file.Path +import dagr.core.cmdline.SystemUtils import dagr.core.config.Configuration import dagr.core.execsystem.{Cores, Memory} import dagr.core.tasksystem.{FixedResources, ProcessTask} @@ -48,6 +49,8 @@ object PicardTask { * @param useAdvancedGcOptions use advanced garbage collection parameters. * @param validationStringency set the default validation stringency for Picard. * @param useAsyncIo true if we are to use asynchronous IO, false otherwise. + * @param useJdkDeflater true if we are to use the JDK deflater, false if we are to use an alternate deflater (Intel). + * @param useJdkInflater true if we are to use the JDK inflater, false if we are to use an alternate inflater (Intel). * @param compressionLevel the compress level to use. * @param createIndex true if we are to create an index, false otherwise. * @param createMd5File true if we are to create an Md5 file, false otherwise. @@ -57,7 +60,8 @@ abstract class PicardTask(var jvmArgs: List[String] = Nil, var useAdvancedGcOptions: Boolean = true, var validationStringency: Option[ValidationStringency] = Some(ValidationStringency.SILENT), var useAsyncIo: Boolean = false, - var useJdkInflater: Option[Boolean] = None, + var useJdkDeflater: Option[Boolean] = Some(!SystemUtils.IntelCompressionLibrarySupported), + var useJdkInflater: Option[Boolean] = Some(!SystemUtils.IntelCompressionLibrarySupported), var compressionLevel: Option[Int] = None, var createIndex: Option[Boolean] = Some(true), var createMd5File: Option[Boolean] = None, @@ -91,6 +95,7 @@ abstract class PicardTask(var jvmArgs: List[String] = Nil, validationStringency.foreach(v => buffer.append("VALIDATION_STRINGENCY=" + v.name())) createIndex.foreach(c => buffer.append("CREATE_INDEX=" + c)) createMd5File.foreach(c => buffer.append("CREATE_MD5_FILE=" + c)) + useJdkDeflater.foreach(u => buffer.append("USE_JDK_DEFLATER=" + u)) useJdkInflater.foreach(u => buffer.append("USE_JDK_INFLATER=" + u)) addPicardArgs(buffer) @@ -116,6 +121,9 @@ abstract class PicardTask(var jvmArgs: List[String] = Nil, this } + /** Sets whether we use the JDK deflater or not. */ + def withJdkDeflater(deflate: Boolean = true) : this.type = { this.useJdkDeflater = Some(deflate); this; } + /** Sets whether we use the JDK inflater or not. */ def withJdkInflater(inflate: Boolean = true) : this.type = { this.useJdkInflater = Some(inflate); this; } } From 5e5481cc7f09ebb44ea65de26222a5b4b0458328 Mon Sep 17 00:00:00 2001 From: Clint Valentine Date: Sun, 5 Apr 2020 20:54:22 -0700 Subject: [PATCH 2/2] Suit @nh13 review --- build.sbt | 2 +- .../dagr/core/cmdline/DagrCoreMain.scala | 42 ------------------- .../scala/dagr/tasks/picard/PicardTask.scala | 6 +-- 3 files changed, 4 insertions(+), 46 deletions(-) diff --git a/build.sbt b/build.sbt index 9d51cafd..59369303 100644 --- a/build.sbt +++ b/build.sbt @@ -123,7 +123,7 @@ lazy val core = Project(id="dagr-core", base=file("core")) .settings(description := "Core methods and classes to execute tasks in dagr.") .settings( libraryDependencies ++= Seq( - "com.fulcrumgenomics" %% "commons" % "1.0.0", + "com.fulcrumgenomics" %% "commons" % "1.1.0-43c062d-SNAPSHOT", "com.fulcrumgenomics" %% "sopt" % "1.0.0", "com.github.dblock" % "oshi-core" % "3.3", "org.scala-lang" % "scala-reflect" % scalaVersion.value, diff --git a/core/src/main/scala/dagr/core/cmdline/DagrCoreMain.scala b/core/src/main/scala/dagr/core/cmdline/DagrCoreMain.scala index 38789b93..7426dda8 100644 --- a/core/src/main/scala/dagr/core/cmdline/DagrCoreMain.scala +++ b/core/src/main/scala/dagr/core/cmdline/DagrCoreMain.scala @@ -43,48 +43,6 @@ import dagr.core.tasksystem.Pipeline import scala.collection.mutable.ListBuffer import scala.util.{Failure, Success} - -/** A little class so that we don't have to rely on SystemUtils from org.apache.commons:commons-lang3. - * - * This implementation is duplicated from `com.fulcrumgenomics.cmdline.SystemUtils`. - */ -private[dagr] object SystemUtils { - /** The OS name prefixes for Linux */ - private val LinuxNamePrefixes: Seq[String] = Seq("Linux", "LINUX") - /** The OS name prefixes for Mac */ - private val MacNamePrefixes: Seq[String] = Seq("Mac") - /** The current OS name. */ - private val OsName: Option[String] = getSystemProperty("os.name") - /** The current OS architecture */ - private val OsArch: Option[String] = getSystemProperty("os.arch") - /** The current OS version */ - private val OsVersion: Option[String] = getSystemProperty("os.version") - - /** Gets a system property. Returns None if not found or not allowed to look at. */ - private def getSystemProperty(property: String): Option[String] = { - try { - Option(System.getProperty(property)) - } catch { case _: SecurityException => None } // not allowed to look at this property - } - - /** True if this OS is Linux, false otherwise. */ - private val IsOsLinux: Boolean = LinuxNamePrefixes.exists(prefix => OsName.exists(_.startsWith(prefix))) - /** True if this OS is Mac, false otherwise. */ - private val IsOsMac: Boolean = MacNamePrefixes.exists(prefix => OsName.exists(_.startsWith(prefix))) - /** Returns true if the architecture is the given name, false otherwise. */ - private def IsOsArch(name: String): Boolean = OsArch.contains(name) - /** Returns true if the operating system version starts with the given version string, false otherwise. */ - private def IsOsVersion(prefix: String): Boolean = OsVersion.exists(_.startsWith(prefix)) - - /** True if the current system supports the Intel Inflater and Deflater, false otherwise. */ - val IntelCompressionLibrarySupported: Boolean = { - if (!SystemUtils.IsOsLinux && !SystemUtils.IsOsMac) false - else if (SystemUtils.IsOsArch("ppc64le")) false - else if (SystemUtils.IsOsMac && SystemUtils.IsOsVersion("10.14.")) false // FIXME: https://github.com/Intel-HLS/GKL/issues/101 - else true - } -} - object DagrCoreMain extends Configuration { /** The packages we wish to include in our command line **/ protected def getPackageList: List[String] = { diff --git a/tasks/src/main/scala/dagr/tasks/picard/PicardTask.scala b/tasks/src/main/scala/dagr/tasks/picard/PicardTask.scala index 9daa039c..0bac2c1e 100644 --- a/tasks/src/main/scala/dagr/tasks/picard/PicardTask.scala +++ b/tasks/src/main/scala/dagr/tasks/picard/PicardTask.scala @@ -25,7 +25,7 @@ package dagr.tasks.picard import java.nio.file.Path -import dagr.core.cmdline.SystemUtils +import com.fulcrumgenomics.commons.util.SystemUtil.IntelCompressionLibrarySupported import dagr.core.config.Configuration import dagr.core.execsystem.{Cores, Memory} import dagr.core.tasksystem.{FixedResources, ProcessTask} @@ -60,8 +60,8 @@ abstract class PicardTask(var jvmArgs: List[String] = Nil, var useAdvancedGcOptions: Boolean = true, var validationStringency: Option[ValidationStringency] = Some(ValidationStringency.SILENT), var useAsyncIo: Boolean = false, - var useJdkDeflater: Option[Boolean] = Some(!SystemUtils.IntelCompressionLibrarySupported), - var useJdkInflater: Option[Boolean] = Some(!SystemUtils.IntelCompressionLibrarySupported), + var useJdkDeflater: Option[Boolean] = Some(!IntelCompressionLibrarySupported), + var useJdkInflater: Option[Boolean] = Some(!IntelCompressionLibrarySupported), var compressionLevel: Option[Int] = None, var createIndex: Option[Boolean] = Some(true), var createMd5File: Option[Boolean] = None,