Skip to content
This repository has been archived by the owner on Apr 24, 2023. It is now read-only.

Change JVM options. remove MaxPermSize and add +UseStringDeduplication #763

Merged
merged 1 commit into from
Mar 20, 2018

Conversation

scrosby
Copy link
Member

@scrosby scrosby commented Mar 13, 2018

Changes proposed in this PR

Switch to G1GC and turn on string deduplication.
Remove deprecated JDK option

Why are we making these changes?

  • In a test with a 500k jobs in queue, there were 92M strings in memory, with the char arrays accounting for a third of the heap.
  • We have been using G1GC internally for a while, so expect that change to be safe.

@shamsimam
Copy link
Contributor

Do you have numbers for the 500K jobs after these changes?

@scrosby
Copy link
Member Author

scrosby commented Mar 14, 2018

Not yet. I'll test and have numbers soon....

@scrosby scrosby requested a review from shamsimam March 14, 2018 18:07
@scrosby
Copy link
Member Author

scrosby commented Mar 14, 2018

Something ate my old response.. Anyways, this code is only used for lein test and internal use. In production, the ansible configuration is used to construct the command line. So (AFAIK), I can't test and give real numbers until we deploy that change and (unless you are aware of a way to do a one-off launch with a custom command line)

@scrosby
Copy link
Member Author

scrosby commented Mar 20, 2018

Turned it on in a test enviornment. With 100k jobs in the queue, the number of char arrays is a third of the number of strings, before they were nearly identical. Memory usage for strings has dropped by a lot. UUID storage now dominates the heap.

@shamsimam shamsimam merged commit 46f0867 into twosigma:master Mar 20, 2018
"-XX:MaxPermSize=500M"
"-XX:+UseG1GC"
"-XX:+UseStringDeduplication"
"-XX:+PrintStringDeduplicationStatistics"
Copy link
Contributor

Choose a reason for hiding this comment

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

I had to comment out -XX:+UseStringDeduplication and -XX:+PrintStringDeduplicationStatistics in order to get my local cook scheduler to start up. Is there something I'm missing @scrosby?

@scrosby scrosby deleted the outgoing/deduplication branch March 22, 2018 18:25
scrosby pushed a commit to scrosby/Cook that referenced this pull request Mar 26, 2018
…plication (twosigma#763)"

Our integration test environment is based around mesos docker 1.3, which
ends up running JDK7, which requires the old options.

This reverts commit 46f0867.
dposada pushed a commit that referenced this pull request Mar 26, 2018
…plication (#763)" (#785)

Our integration test environment is based around mesos docker 1.3, which
ends up running JDK7, which requires the old options.

This reverts commit 46f0867.
dposada pushed a commit to dposada/Cook that referenced this pull request Nov 1, 2018
…plication (twosigma#763)" (twosigma#785)

Our integration test environment is based around mesos docker 1.3, which
ends up running JDK7, which requires the old options.

This reverts commit 46f0867.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants