Align env var lookup in clrconfig and jithost #77025
Conversation
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
ghost
added
the
community-contribution
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsClrConfig checks for
|
am11
force-pushed
the
feature/superpmi/DOTNET_<env-vars>
branch
from
36fe408
to
8c649d0
Compare
| { | ||
| static const WCHAR Prefix[] = W("COMPlus_"); | ||
| static const size_t PrefixLen = (sizeof(Prefix) / sizeof(Prefix[0])) - 1; | ||
| #include <clrconfignocache.h> |
There was a problem hiding this comment.
As a nit, this perhaps should be in runtimedetails.h, but I guess this is ok.
There was a problem hiding this comment.
Yup, it's an existing header, which I have reused (to avoid redefining consts and lengths).
@AaronRobinsonMSFT, ideally we could have a unified API with flag-y options to avoid duplicate functions. The usage of such API in jisthost.cpp would look like:
ClrConfig::Get(key, &returnBuffer, (ClrConfig_USEPREFIX & ~ClrConfig_USECACHE));
Having different classes for cache vs. nochace is somewhat verbose.
There was a problem hiding this comment.
@AaronRobinsonMSFT, ideally we could have a unified API with flag-y options to avoid duplicate functions. The usage of such API in jisthost.cpp would look like:
Sure, but that doesn't change the reason that the nocache version exists. Note that the nocache doesn't do any memory management nor does it work with WCHAR, it is entirely for char. This was done due to the various places that can't include or accept the lifetime management nor the requirement of chaining in the PAL. We can move the nocache, but it has broader build constraints about where it is used and done in the way it is to reduce that duplication.
ClrConfig checks for
DOTNET_prefix and fallback toCOMPlus_for environment variable lookup. This patch updates the lookup in superpmi to use the same order.