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.

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

Overriding stack size (-Xss) fails except via SBT_OPTS #5181

Closed
sloshy opened this issue Oct 10, 2019 · 5 comments · Fixed by sbt/sbt-launcher-package#346
Closed

Overriding stack size (-Xss) fails except via SBT_OPTS #5181

sloshy opened this issue Oct 10, 2019 · 5 comments · Fixed by sbt/sbt-launcher-package#346
Labels
area/runner-script issues around sbt-the-bash-script, or bat script Bug
Milestone

Comments

@sloshy
Copy link

sloshy commented Oct 10, 2019

steps

sbt version: 1.3.2

problem

Overriding the stack size via .jvmopts, or the -J flag fails. Stack size is set to the default 4M value even when overridden.

expectation

Overriding the stack size by itself should work no matter the method.

notes

Using SBT_OPTS works correctly. I've traced this down to the launcher script for sbt and have identified a workaround for the time being. If you specify a memory option (-Xm*) then the portion of code in the launcher script that sets the stack size will not be called.

A couple ideas on how this could be fixed:

  1. Add a rule that selectively overrides -Xss if the argument is present when launching sbt
  2. Apply java arguments after the defaults so that the defaults are only selectively overridden (this is the behavior for SBT_OPTS currently when changing the stack size)

To reproduce: open any SBT project and set the stack size via -J or .jvm/sbtopts. Run sbt -d to see the order of JVM args as they are applied. Note that the stack size parameter is applied before the defaults which override it. Now set it via SBT_OPTS and see that your option is specified after the default argument, overriding it.

@sloshy sloshy added the Bug label Oct 10, 2019
@er1c er1c added the area/runner-script issues around sbt-the-bash-script, or bat script label Oct 11, 2019
@er1c
Copy link
Contributor

er1c commented Oct 11, 2019

I'll admit I was a bit confused on how this behavior should work, when I was doing the windows sbt.bat port.

All of the memory arguments probably need a better set of tests to help explicitly document how the memory overrides should interact with each other (https://github.com/sbt/sbt-launcher-package/blob/master/integration-test/src/test/scala/RunnerTest.scala) - and those tests themselves probably need to be discussed if they are actually desireable, since it's not obvious to me what should be overridden when.

The comment here is especially confusing with how the overrides should work (or as it's written, the default memory settings SHOULD NOT be getting applied if you have a -Xss set): https://github.com/sbt/sbt-launcher-package/blob/master/src/universal/bin/sbt#L177-L178

  # if we detect any of these settings in ${JAVA_OPTS} or ${JAVA_TOOL_OPTIONS} we need to NOT output our settings.
  # The reason is the Xms/Xmx, if they don't line up, cause errors.

If they are, then definitely a bug.

@eed3si9n
Copy link
Member

I agree it's a bug.

We do check for heap size related settings to prevent conflict with user-provided parameters, but stack size setting -Xss is not being checked.

@er1c
Copy link
Contributor

er1c commented Dec 29, 2020

Similar/same issue -Xss4M gets added with specifying -mem and duplicates -Xss6M:

> cat .jvmopts
cat .jvmopts
-Dfile.encoding=UTF8
-Xms1G
-Xmx5G
-Xss6M
-XX:ReservedCodeCacheSize=512m
-XX:+UseG1GC

❯ sbt -d -mem 5632 'testOnly foo'
[addSbt] arg = '-debug'
[addMemory] arg = '5632'
[addJava] arg = '-Xms5632m'
[addJava] arg = '-Xmx5632m'
[addJava] arg = '-Xss4M'
[addJava] arg = '-XX:ReservedCodeCacheSize=512m'
...
[sbt_options] declare -a sbt_options=()
[process_args] java_version = '8'
# Executing command line:
java
-Dfile.encoding=UTF8
-Xss6M
-XX:+UseG1GC
-Xms5632m
-Xmx5632m
-Xss4M
-XX:ReservedCodeCacheSize=512m
-jar
/Users/eric/.sdkman/candidates/sbt/1.4.6/bin/sbt-launch.jar
-debug

@ericdotdata
Copy link
Contributor

Pending fix: sbt/sbt-launcher-package#346

@eed3si9n
Copy link
Member

Fixed in sbt/sbt-launcher-package#346 by @ericdotdata

eed3si9n pushed a commit to eed3si9n/sbt that referenced this issue Apr 20, 2021
eed3si9n added a commit to eed3si9n/sbt that referenced this issue Apr 20, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/runner-script issues around sbt-the-bash-script, or bat script Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants