-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Enable overriding default stack size on Unix #13517
Enable overriding default stack size on Unix #13517
Conversation
@swgillespie I have noticed that after I've updated the config knobs doc using the script, some of the GC related knobs were removed. Namely the UNSUPPORTED_GCtraceStart, UNSUPPORTED_GCtraceEnd, UNSUPPORTED_GCprnLvl and INTERNAL_GCtraceFacility knobs that are referred to in the eeconfig.cpp under the TRACE_GC ifdef, but they are not defined in the src/inc/clrconfigvalues.h. Are these really dead? |
src/pal/src/init/pal.cpp
Outdated
} | ||
|
||
uint8_t *s = (uint8_t *)_alloca(g_defaultStackSize); | ||
s[0] = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decent compiler will remove this with optimizations on. You may want to do this with volatile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is marked with attribute optnone, so no optimizations are allowed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It maybe nice to move the small part that does the alloca moved into separate method and mark just that - more readable, and probably smaller too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I actually had the function like that until I've added the ability to set the size by the env variable.
src/pal/src/init/pal.cpp
Outdated
g_defaultStackSize = 1536 * 1024; | ||
} | ||
|
||
uint8_t *s = (uint8_t *)_alloca(g_defaultStackSize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this going to commit the memory (touch every page out of the 1.5MB) or is this going to touch the bottom one only? It is pretty unfortunate if this is committing 1.5MB - most of it won't be used by the process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't commit the whole range. Or at least doesn't grow the resident set for the whole range. I've added dumping the "VmRSS" field from the /proc/self/status and the value doesn't change. I've also tried to touch the whole range page by page and the resident set was growing in steps of 264kB every 66 pages I've touched.
Alpine doesn't keep any fixed stack limit for the main thread. The pthread_getattr_np on Alpine gets the stack size by scanning pages from the bottom of the stack until it finds the stack guard.
Alpine Linux has a very small default stack size limit, about 80kB. This is not enough for running coreclr apps. This change enables overriding the default stack size using the COMPlus_DefaultStackSize env variable. For Alpine, it also sets the default stack size to the same value we use for Windows, which is 1.5MB.
1447449
to
eee9694
Compare
@jkotas I've reflected your feedback and amended my commit. |
@dotnet-bot test Tizen armel Cross Release Build please |
Alpine Linux has a very small default stack size limit, about 80kB.
This is not enough for running coreclr apps.
This change enables overriding the default stack size using the
COMPlus_DefaultStackSize env variable. For Alpine, it also sets
the default stack size to the same value we use for Windows, which
is 1.5MB.
I have also added documentation for this env variable and also the other pre-existing env variables used by the crashdump to the Documentation/project-docs/clr-configuration-knobs.md. Since that file is generated by the Documentation/project-docs/clr-complus-conf-docgen.sh, I have modified that script and added the new resulting clr-configuration-knobs.md. The script was ran more than a year ago and so there were some stale and some new configuration knobs.