search

Metacello / metacello

Metacello is a package management system for Smalltalk
MIT License
87 stars 43 forks source link

[Squeak-only] Fix performance regression for projects that have an empty repoPath #523

Closed tom95 closed 4 years ago

tom95 commented 4 years ago

If a project had an empty repoPath, we tried to re-download the whole package after each cache flush (and thus in particular after each image restart). With this change we now explicitly remember that we have inspected and learned the correct repoPath.

Symptoms of this were for example the Monticello Browser taking very long to open after an image restart.

tom95 commented 4 years ago

@krono Thank you for the thorough review. I had made a false assumption that repoPath can appear as the empty string after parsing the repository URL, which would have made the new instance variable a necessity. After stepping through the code I realized that it will always be either nil or non-empty string, meaning the complexity was not necessary. I updated the PR accordingly.

The downloadJSONTags method should have been in a separate commit. I would suggest to merge it as part of this PR, but mention it in the merge commit message, maybe like so:

Fix performance regression for projects that have an empty repoPath; respect etags for Github downloads

- If a project had an empty repoPath, we tried to re-download the whole package after each cache flush (and
  thus in particular after each image restart). With this change we now explicitly remember that we have
  inspected and learned the correct repoPath
- MCFetchGithubRepository should override downloadJSONTags and provide the etag of the last download
  (copied from original GithubRepository)

Travis appears to have had hickups because of the libpulse-simple.so issue. I had the tests run locally using smalltalkCI where everything came out green.