stevedlawrence commented on code in PR #210:
URL: https://github.com/apache/daffodil-sbt/pull/210#discussion_r3751241055
##########
src/main/scala/org/apache/daffodil/DaffodilPlugin.scala:
##########
@@ -518,14 +518,30 @@ object DaffodilPlugin extends AutoPlugin {
/**
* JVM options used for the forked process to build saved parsers
*
- * Defaults to just setting various system properties to configure loggers
that might be
- * used by different daffodil versions
+ * Defaults to setting various system properties to configure loggers that
might be used by
+ * different daffodil versions, plus the -Xss this sbt JVM was started
with, if any. Since
+ * sbt users often already tune sbt's own stack size (e.g. via .sbtopts or
.jvmopts),
+ * it's convenient to reuse that value for the forked process rather than
requiring it to
+ * be configured again. If a project wants a different stack size just for
this forked
+ * process, it can append its own -Xss, e.g.:
+ *
+ * packageDaffodilBin / javaOptions += "-Xss64m"
+ *
+ * Note this relies on the JVM using the last of multiple -Xss arguments
it is given, so an
+ * appended -Xss here correctly takes precedence over the inherited one
below.
*/
- packageDaffodilBin / javaOptions := Seq(
- s"-Dorg.slf4j.simpleLogger.logFile=System.out",
- s"-Dorg.slf4j.simpleLogger.defaultLogLevel=${(packageDaffodilBin /
logLevel).value}",
- s"-Dorg.apache.logging.log4j.level=${(packageDaffodilBin /
logLevel).value}"
- ),
+ packageDaffodilBin / javaOptions := {
+ val inheritedXss =
+
java.lang.management.ManagementFactory.getRuntimeMXBean.getInputArguments
+ .toArray(Array.empty[String])
+ .find(_.startsWith("-Xss"))
Review Comment:
Should this be `findLast`, since I think JVM's if an option is specified
multiple times the JVM uses the last one?
##########
src/main/scala/org/apache/daffodil/DaffodilPlugin.scala:
##########
@@ -518,14 +518,30 @@ object DaffodilPlugin extends AutoPlugin {
/**
* JVM options used for the forked process to build saved parsers
*
- * Defaults to just setting various system properties to configure loggers
that might be
- * used by different daffodil versions
+ * Defaults to setting various system properties to configure loggers that
might be used by
+ * different daffodil versions, plus the -Xss this sbt JVM was started
with, if any. Since
+ * sbt users often already tune sbt's own stack size (e.g. via .sbtopts or
.jvmopts),
+ * it's convenient to reuse that value for the forked process rather than
requiring it to
+ * be configured again. If a project wants a different stack size just for
this forked
+ * process, it can append its own -Xss, e.g.:
+ *
+ * packageDaffodilBin / javaOptions += "-Xss64m"
+ *
+ * Note this relies on the JVM using the last of multiple -Xss arguments
it is given, so an
+ * appended -Xss here correctly takes precedence over the inherited one
below.
*/
- packageDaffodilBin / javaOptions := Seq(
- s"-Dorg.slf4j.simpleLogger.logFile=System.out",
- s"-Dorg.slf4j.simpleLogger.defaultLogLevel=${(packageDaffodilBin /
logLevel).value}",
- s"-Dorg.apache.logging.log4j.level=${(packageDaffodilBin /
logLevel).value}"
- ),
+ packageDaffodilBin / javaOptions := {
+ val inheritedXss =
+
java.lang.management.ManagementFactory.getRuntimeMXBean.getInputArguments
+ .toArray(Array.empty[String])
+ .find(_.startsWith("-Xss"))
+
+ Seq(
+ s"-Dorg.slf4j.simpleLogger.logFile=System.out",
+ s"-Dorg.slf4j.simpleLogger.defaultLogLevel=${(packageDaffodilBin /
logLevel).value}",
+ s"-Dorg.apache.logging.log4j.level=${(packageDaffodilBin /
logLevel).value}"
+ ) ++ inheritedXss
Review Comment:
I'm hesitant about this change.
I would think in the majority of cases a large stack size is really only
needed for schema compilation and so it's better that to only apply Xss to
`packageDaffodilBin / javaOptions`. This keeps the sbt stack small and also
sort of self documents that schema compilation needs a certain stack size.
Similarly for options like -Xmx which I think some schemas also need for
compilation.
I think this is also isn't really consistent with how SBT works, so it's
kind of a non-standard and surprising behavior. In testing I've done, it looks
like forking tasks (e.g tests/run) fork using default JVM options, it doesn't
assume that options provided to SBT also make sense for other tasks. In the
packageDaffodilBin case, thatmight be true, but it isn't necessarily true, and
it feels better to be explicit about it by setting `packageDaffodilBin /
javaOptions`.
I don't feel super strongly about it, so if this really makes things easier
I won't block it.
Note that if this is the convention we do decide to use, we should probably
also copy things like -Xmx and maybe some others, since I imagine most cases we
need more than just a bigger stack size for
compiling large schemas.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
