Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Cache] Optimize caching of tags#45896

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@sbelyshkin
Copy link
Contributor

@sbelyshkinsbelyshkin commentedMar 30, 2022
edited
Loading

QA
Branch?6.1
Bug fix?no
New feature?no
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

It's the follow-up to#42997.

  1. Forcing the adapter to fetch tags on item commits makes the algorithm straightforward and a bit more optimized.
  2. Caching tag versions only when they are retrieved from or being persisted to the pool ensures that any tagged item will be rejected when there is no related tags in the pool.
  3. Using FIFO instead of LRU for known tag versions allows to use all cached tags until expiration and still be able to prevent memory leak.

@carsonbot
Copy link

It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you.

Cheers!

Carsonbot

@nicolas-grekas
Copy link
Member

Thanks for having another look :)
For 1. and 2., is it really better? To me, the main thing to optimize is the number of roundtrips to the backend. The changes here increase the number of roundtrips, so they should degrade perf to me.

@sbelyshkin
Copy link
ContributorAuthor

sbelyshkin commentedMar 31, 2022
edited
Loading

P.1 and p.2 don't change the number of roundtrips, they just force the adapter to always work the same and not to rely on quasi-uniqueness of tag versions in some situations. P.3 may change the number of roundtrips but only downward by allowing large sets of tags (500+) to be cached for the very next reads (until other tags are requested). And although smaller sets are not affected much, I would suggest to use pruning instead of LRU cut-off. Pruning only expired tags would allow use of all known tags in any read operaion during their TTLs (as it was before).

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

Here is my (miss)understanding of your proposal :)

@sbelyshkinsbelyshkinforce-pushed theimprove-tags-fetching branch 2 times, most recently from78fff0c to7d0735cCompareApril 8, 2022 11:25
@sbelyshkinsbelyshkin changed the title[Cache] Optimize fetching of tags[Cache] Optimize caching of tagsApr 15, 2022
@sbelyshkinsbelyshkinforce-pushed theimprove-tags-fetching branch 2 times, most recently from9763ed0 to1ccd607CompareApril 18, 2022 05:40
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

Just one last question and good to me!

@sbelyshkinsbelyshkinforce-pushed theimprove-tags-fetching branch 3 times, most recently from82ca189 to0faaa75CompareApril 25, 2022 03:08
@nicolas-grekas
Copy link
Member

Thank you@sbelyshkin.

@nicolas-grekasnicolas-grekas merged commitfa636c0 intosymfony:6.1Apr 25, 2022
@fabpotfabpot mentioned this pull requestApr 27, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

6.1

Development

Successfully merging this pull request may close these issues.

3 participants

@sbelyshkin@carsonbot@nicolas-grekas

[8]ページ先頭

©2009-2025 Movatter.jp