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

Conversation

@rmshaffer
Copy link
Contributor

@rmshaffer rmshaffer commented Sep 18, 2020

This is a fix for what seems to be a possible race condition while loading packages.

We were seeing frequent Katas validation failures in the E2E build due to the assemblies from the Katas package not being available, even though the %package Microsoft.Quantum.Katas cell in the notebook had already completed successfully.

With this change, so far the Katas validation in the E2E build has passed 6/6 times, and considering we were seeing a failure rate that seemed to be nearly 50% recently, I think it's worth merging this fix to get further testing. I don't have high confidence that this actually fixes the issue, but it's a safe change and can't hurt anything.

@rmshaffer rmshaffer changed the title [DRAFT] Synchronize package loading Avoid possible race condition during package loading Sep 18, 2020
@rmshaffer rmshaffer requested review from anpaz and cgranade September 18, 2020 14:19
Copy link
Contributor

@cgranade cgranade left a comment

Choose a reason for hiding this comment

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

Looks good to me. Have you been able to verify if there are there any performance implications to this solution?

@rmshaffer
Copy link
Contributor Author

Looks good to me. Have you been able to verify if there are there any performance implications to this solution?

Good question. The fact that the E2E builds are succeeding is an indication that it doesn't make things too much worse. (Previously when loading the Microsoft.Quantum.Standard.Visualization package synchronously on load, it caused kernel startup timeouts during Katas validation. So at least we're not hitting that problem.)

In principle this shouldn't affect performance much. In particular, this change doesn't serialize the dependency resolution or downloading portions of package loading, which are the potentially time-consuming parts. It only serializes the updates to the cached lists of packages and assemblies. That certainly takes some time, as these are long lists, but in practice I wouldn't think this will cause a noticeable difference (my rough guess would be sub-second impact to kernel startup time, and only in cases where a notebook/script/project is actually loading additional packages at startup, as in the case of the Katas notebooks).

@rmshaffer rmshaffer merged commit d75752a into main Sep 18, 2020
@rmshaffer rmshaffer deleted the rmshaffer/package-sync branch September 18, 2020 15:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants