Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Lighting up Shared packages#1330

Merged
ramarag merged 4 commits into
dotnet:masterfrom
ramarag:shared_packages
Feb 3, 2017
Merged

Lighting up Shared packages#1330
ramarag merged 4 commits into
dotnet:masterfrom
ramarag:shared_packages

Conversation

@ramarag
Copy link
Copy Markdown
Member

@ramarag ramarag commented Jan 27, 2017

@gkhanna79 @schellap @eerhardt PTAL

This is a port of #1109

@ramarag
Copy link
Copy Markdown
Member Author

ramarag commented Jan 27, 2017

FYI @pakrym @muratg @glennc - DOTNET_HOSTING_OPTIMIZATION_CACHE is going away to be replaced with Design Doc on dotnet cache

schellap
schellap reviewed Jan 27, 2017
View reviewed changes
Copy link
Copy Markdown

@schellap schellap left a comment

Choose a reason for hiding this comment

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

Just the minor stuff above. LGTM! Hope you've tested with your recent changes to publish profile and cache command!

Comment thread src/corehost/cli/args.h
probe_config_t(
const pal::string_t& probe_dir,
bool match_hash,
bool patch_roll_fwd,

This comment was marked as spam.

Sign in to view

Comment thread src/corehost/cli/args.h Outdated
}

static probe_config_t cache(const pal::string_t& dir)
{

This comment was marked as spam.

Sign in to view

Comment thread src/corehost/cli/libhost.h

make_clrstr_arr(input->config_keys.len, input->config_keys.arr, &init->cfg_keys);
make_clrstr_arr(input->config_values.len, input->config_values.arr, &init->cfg_values);
if (input->version_lo >= offsetof(host_interface_t, host_mode) + sizeof(input->host_mode))

This comment was marked as spam.

Sign in to view

Comment thread src/corehost/cli/libhost.h
}

if (input->version_lo >= offsetof(host_interface_t, tfm) + sizeof(input->tfm))
{

This comment was marked as spam.

Sign in to view

Comment thread src/corehost/common/pal.unix.cpp Outdated
{
return false;
}
append_path(&dir, _X(".dotnet"));

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread src/corehost/common/pal.unix.cpp Outdated
return false;
}
pal::string_t dir = get_directory(dotnet_exe);
append_path(&dir, _X("packages"));

This comment was marked as spam.

Sign in to view

@muratg
Copy link
Copy Markdown

muratg commented Jan 27, 2017

@ramarag @schellap if the env var is removed, how will dotnet find where the optimization cache is on the disk? This is a major scenario for Azure App Service (Antares) deployments to improve startup time for App Service apps.

cc @DamianEdwards

@DamianEdwards
Copy link
Copy Markdown
Member

DamianEdwards commented Jan 27, 2017

@muratg this replaces that. This is effectively a new cache that's more formally supported and achieves what the existing optimization cache did plus more, e.g. reducing need to deploy the assemblies with the app at all.

@muratg
Copy link
Copy Markdown

muratg commented Jan 27, 2017 •
edited
Loading

I just read the spec. It looks like there will be a fixed global location for the ngen'd files (plus a "local" one.) So no way to configure the location of the global one? I wonder if we may want to specify the location still.

@DamianEdwards
Copy link
Copy Markdown
Member

DamianEdwards commented Jan 27, 2017

@muratg by default it would be located next to the instance of dotnet.exe being invoked (aka the global location). On Antares, that could (should) be on the fast drive. If that's not enough, then yes it should be possible to set this location via an environment variable/cmd line arg.

@schellap
Copy link
Copy Markdown

schellap commented Jan 30, 2017 •
edited
Loading

@muratg, paths can be chained within $DOTNET_SHARED_PACKAGES env. This is in the spec in host probe precedence. The architecture of the host process will be appended to these paths to look for packages.

@ramarag, it might be better to correct the spec to include this and also change the probe order to reflect implementation.

@ramarag ramarag changed the title [WIP] Shared packages Lighting up Shared packages Feb 1, 2017
@ramarag ramarag mentioned this pull request Feb 1, 2017
Closed
@ramarag ramarag self-assigned this Feb 2, 2017
@ramarag
Copy link
Copy Markdown
Member Author

ramarag commented Feb 3, 2017

i am merging this PR for now, will followup if any new new comments become relevant

@ramarag ramarag merged commit 18bdee5 into dotnet:master Feb 3, 2017
eerhardt added a commit to eerhardt/core-setup that referenced this pull request Mar 31, 2017
@eerhardt
Use correct package cache environment variable name.
5d6b1ec 
Fix #622.

Also remove hash check for loading assemblies from the package cache.  This matches the 2.0 dotnet host behavior changed in dotnet#1330.
@eerhardt eerhardt mentioned this pull request Mar 31, 2017
Merged
@natemcmaster natemcmaster mentioned this pull request Apr 3, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@schellap schellap schellap left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@ramarag ramarag

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ramarag @muratg @DamianEdwards @schellap @dnfclas