Skip to content

[Cache] improve perf of pruning for fs-based adapters #33921

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

Merged
merged 1 commit into from
Oct 9, 2019

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 4.4
Bug fix? no
New feature? no
Deprecations? no
Tickets Fix #32701
License MIT
Doc PR -

Thank you Blackfire.io

https://blackfire.io/profiles/compare/8044c3cf-1541-4a24-9804-5670c63fcb4c/graph

image

image

@nicolas-grekas nicolas-grekas added this to the next milestone Oct 9, 2019
@nicolas-grekas nicolas-grekas force-pushed the cache-prune-perf branch 2 times, most recently from 658fafb to c40342b Compare October 9, 2019 10:15
@nicolas-grekas nicolas-grekas force-pushed the cache-prune-perf branch 2 times, most recently from 98cc70c to 416ae6a Compare October 9, 2019 10:34
@nicolas-grekas nicolas-grekas force-pushed the cache-prune-perf branch 5 times, most recently from da50115 to 2029726 Compare October 9, 2019 14:10
nicolas-grekas added a commit that referenced this pull request Oct 9, 2019
…olas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Cache] improve perf of pruning for fs-based adapters

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #32701
| License       | MIT
| Doc PR        | -

Thank you [Blackfire.io](https://blackfire.io)

https://blackfire.io/profiles/compare/8044c3cf-1541-4a24-9804-5670c63fcb4c/graph

![image](https://user-images.githubusercontent.com/243674/66465435-22f9fc80-ea81-11e9-83fc-2444434a33e6.png)

![image](https://user-images.githubusercontent.com/243674/66465462-2d1bfb00-ea81-11e9-827a-4c0502cb5f4a.png)

Commits
-------

0302b1d [Cache] improve perf of pruning for fs-based adapters
@nicolas-grekas nicolas-grekas merged commit 0302b1d into symfony:4.4 Oct 9, 2019
@nicolas-grekas nicolas-grekas deleted the cache-prune-perf branch October 9, 2019 14:30
@Toflar
Copy link
Contributor

Toflar commented Oct 12, 2019

Thank you @nicolas-grekas!! <3

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
fabpot added a commit that referenced this pull request Jan 10, 2020
…ory when there are no namespace (fancyweb)

This PR was merged into the 4.4 branch.

Discussion
----------

[Filesystem][FilesystemCommonTrait] Use a dedicated directory when there are no namespace

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This PR fixes the following edge case:
- you use a namespaced filesystem adapter with root dir `/foo` and namespace `a`: all files are written in `/foo/a` (eg: it will write `/foo/a/b/file`)
- you use another filesystem adapter with the same root dir but without namespace and you clear it.
- it will fail because it will try to delete the `/foo/a/b` directory (see #33921 new `scanHashDir()` method - `a` is a possible dir value (see `getFile()`) so we look for it).

The simple solution (suggested by @nicolas-grekas) is to put the "empty namespace" in a dedicated directory.

Bonus: that will fix the tests that currently always fail on AppVeyor. The file in the namespace `a` is a leftover from a previous test. Without this patch, I have the same fail on my Mac. I did not look why it currently passes on travis.

Commits
-------

eaa767b [Filesystem][FilesystemCommonTrait] Use a dedicated directory when there are no namespace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Cache] Purging takes forever with new FilesystemTagAwareAdapter
5 participants