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

Use a persistent cache for FMU contents #30

Merged
merged 11 commits into from
Apr 1, 2020

Conversation

kyllingstad
Copy link
Member

@kyllingstad kyllingstad commented Sep 30, 2019

Fixes #11.

I've submitted a companion pull request for cse-core which must be merged first: open-simulation-platform/libcosim#388. Once that is accepted, I'll revert the channel change in conanfile.txt here.

@kyllingstad kyllingstad added the enhancement New feature or request label Sep 30, 2019
@kyllingstad kyllingstad self-assigned this Sep 30, 2019
hplatou
hplatou previously approved these changes Oct 1, 2019
Copy link

@hplatou hplatou left a comment

Choose a reason for hiding this comment

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

Nice! Should see an effect on the 90+GB cached directories on the Jenkins slaves :)

conanfile.txt Outdated Show resolved Hide resolved
@kyllingstad
Copy link
Member Author

Closing for the time being. Will pick up again later.

@kyllingstad
Copy link
Member Author

Reopening now that open-simulation-platform/libcosim#388 is ready.

@kyllingstad kyllingstad reopened this Jan 14, 2020
@kyllingstad kyllingstad requested a review from hplatou January 14, 2020 09:25
@kyllingstad kyllingstad mentioned this pull request Feb 25, 2020
@kyllingstad kyllingstad dismissed hplatou’s stale review March 27, 2020 11:46

Lots of changes since the approval was first given.

@kyllingstad
Copy link
Member Author

I just discovered that this PR had bitrotted. I've updated it now, so it's ready for a new review. It's a rather long-standing one, so it would be good to get it done. :)

@ljamt
Copy link
Member

ljamt commented Mar 30, 2020

This PR looks good to me. I've also tested it functionally. The cached fmus end up in localAppData/cse as expected, and the ´clean-cache` subcommand clears it.

When running the dp-ship example or inspecting the OSOM.fmu, I notice that we still create a "cse_guid" folder in localAppData/Temp. I think the importer in cse-core still extracts the fmu to the localAppData/Temp folder. Is it supposed to work like this?

@kyllingstad
Copy link
Member Author

When running the dp-ship example or inspecting the OSOM.fmu, I notice that we still create a "cse_guid" folder in localAppData/Temp. I think the importer in cse-core still extracts the fmu to the localAppData/Temp folder. Is it supposed to work like this?

Weird, I can't reproduce this on my computer, neither on Windows nor Linux. Are you able to run cse inspect via a debugger and see when the folder gets created?

@ljamt
Copy link
Member

ljamt commented Mar 31, 2020

Debugged inspect with different FMUs now.
An empty cse_guid folder is created in localAppData/Temp when line inspect.cpp line 72 is executed:

const auto uriReference = to_uri(args["uri_or_path"].as<std::string>());

This folder contains the extracted fmu after line 73 has executed:
const auto uriResolver = caching_model_uri_resolver();

All folders are deleted once the cse inspect command returns.

Sorry. Debugging outdated code. Will come back with more accurate info

@ljamt
Copy link
Member

ljamt commented Mar 31, 2020

It is inpect.cpp line 74:
const auto model = uriResolver->lookup_model(baseUri, uriReference);
A cse_guid is created and contains only the ModelDescription.xml and is destroyed when the lookup_model returns.

Could it be the peek_model_description in importer.cpp?

@kyllingstad
Copy link
Member Author

Could it be the peek_model_description in importer.cpp?

Yes. That is as expected, and as long as the directory gets deleted again afterwards, it's not a bug. Thanks for checking!

@kyllingstad kyllingstad merged commit 786896f into master Apr 1, 2020
@kyllingstad kyllingstad deleted the feature/11-persistent-fmu-cache branch April 1, 2020 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use a persistent FMU cache
3 participants