Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2296913003:
[WeakMemoryCache] Make references from MemoryCache to Resource Weak (Closed)
|
Created:
4 years, 3 months ago by hiroshige Modified:
4 years, 3 months ago CC:
chromium-reviews, blink-reviews, loading-reviews+fetch_chromium.org, tyoshino+watch_chromium.org, Nate Chapin, gavinp+loader_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@WeakMemoryCache_RemoveStats Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[WeakMemoryCache] Make references from MemoryCache to Resource Weak
Design doc (Step 1):
https://docs.google.com/document/d/1Pj4vzxzCKC_11SugdJ_x7wq7ErE-pHtf4w4FLKdcF7g/edit?usp=sharing
This cl relands
- https://codereview.chromium.org/1915113005
- https://codereview.chromium.org/2066763002
except for code related to Field-Trial or SharedBuffer::unlock().
BUG=603462
Committed: https://crrev.com/581fd42e219de5510f0fd7729e7b00a411ff68a8
Cr-Commit-Position: refs/heads/master@{#417245}
Patch Set 1 #Patch Set 2 : Assume resource is always non-null #
Depends on Patchset: Messages
Total messages: 25 (10 generated)
Description was changed from ========== [WeakMemoryCache] Make references from MemoryCache to Resource Weak Reland of: - https://codereview.chromium.org/1915113005 - https://codereview.chromium.org/2066763002 except for code related to Field-Trial or SharedBuffer::unlock(). BU=603462 ========== to ========== [WeakMemoryCache] Make references from MemoryCache to Resource Weak Reland of: - https://codereview.chromium.org/1915113005 - https://codereview.chromium.org/2066763002 except for code related to Field-Trial or SharedBuffer::unlock(). BUG=603462 ==========
Description was changed from ========== [WeakMemoryCache] Make references from MemoryCache to Resource Weak Reland of: - https://codereview.chromium.org/1915113005 - https://codereview.chromium.org/2066763002 except for code related to Field-Trial or SharedBuffer::unlock(). BUG=603462 ========== to ========== [WeakMemoryCache] Make references from MemoryCache to Resource Weak Design doc (Step 1): https://docs.google.com/document/d/1Pj4vzxzCKC_11SugdJ_x7wq7ErE-pHtf4w4FLKdcF... This cl relands - https://codereview.chromium.org/1915113005 - https://codereview.chromium.org/2066763002 except for code related to Field-Trial or SharedBuffer::unlock(). BUG=603462 ==========
hiroshige@chromium.org changed reviewers: + japhet@chromium.org, kinuko@chromium.org, yhirano@chromium.org
Let's make MemoryCache weak on ToT. PTAL.
lgtm
csharrison@chromium.org changed reviewers: + csharrison@chromium.org
Should we FYI loading-dev about this change?
On 2016/09/02 12:12:50, Charlie Harrison wrote: > Should we FYI loading-dev about this change? Added a comment about this CL in the Weak MemoryCache thread on loading-dev.
lgtm I don't know how to you registerWeakMembers, though. It might be good to get reviewed by an oilpan expert.
On 2016/09/06 05:48:53, yhirano wrote: > lgtm > > I don't know how to you registerWeakMembers, though. It might be good to get > reviewed by an oilpan expert. s/how to you/how to use/g
lgtm, though I too don't know WeakMembers well enough to be confident that this usage is correct.
According to comments about registerWeakMembers(), I uploaded Patch Set 3 that uses prefinalizer of Resource instead of registerWeakMembers(). How about this? (The purpose of the prefinalizer (and registerWeakMembers() in previous patch sets) is, by calling MemoryCache::remove(), to keep the MemoryCache's live/dead sizes to be consistent to avoid wrong eviction behavior based on stale value of current cache size.)
On 2016/09/07 02:39:28, hiroshige wrote: > According to comments about registerWeakMembers(), I uploaded Patch Set 3 that > uses prefinalizer of Resource instead of registerWeakMembers(). How about this? > (The purpose of the prefinalizer (and registerWeakMembers() in previous patch > sets) is, by calling MemoryCache::remove(), to keep the MemoryCache's live/dead > sizes to be consistent to avoid wrong eviction behavior based on stale value of > current cache size.) IIUC the weak member in MemoryCacheEntry is swept before the pre-finalization step, so you can't find an entry in willDestroyResource.
On 2016/09/07 03:43:36, yhirano wrote: > On 2016/09/07 02:39:28, hiroshige wrote: > > According to comments about registerWeakMembers(), I uploaded Patch Set 3 that > > uses prefinalizer of Resource instead of registerWeakMembers(). How about > this? > > (The purpose of the prefinalizer (and registerWeakMembers() in previous patch > > sets) is, by calling MemoryCache::remove(), to keep the MemoryCache's > live/dead > > sizes to be consistent to avoid wrong eviction behavior based on stale value > of > > current cache size.) > > IIUC the weak member in MemoryCacheEntry is swept before the pre-finalization > step, so you can't find an entry in willDestroyResource. Oh, thanks. I'll remove Patch Set 3 and land Patch Set 2.
Patchset #3 (id:40001) has been deleted
The CQ bit was checked by hiroshige@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by hiroshige@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [WeakMemoryCache] Make references from MemoryCache to Resource Weak Design doc (Step 1): https://docs.google.com/document/d/1Pj4vzxzCKC_11SugdJ_x7wq7ErE-pHtf4w4FLKdcF... This cl relands - https://codereview.chromium.org/1915113005 - https://codereview.chromium.org/2066763002 except for code related to Field-Trial or SharedBuffer::unlock(). BUG=603462 ========== to ========== [WeakMemoryCache] Make references from MemoryCache to Resource Weak Design doc (Step 1): https://docs.google.com/document/d/1Pj4vzxzCKC_11SugdJ_x7wq7ErE-pHtf4w4FLKdcF... This cl relands - https://codereview.chromium.org/1915113005 - https://codereview.chromium.org/2066763002 except for code related to Field-Trial or SharedBuffer::unlock(). BUG=603462 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [WeakMemoryCache] Make references from MemoryCache to Resource Weak Design doc (Step 1): https://docs.google.com/document/d/1Pj4vzxzCKC_11SugdJ_x7wq7ErE-pHtf4w4FLKdcF... This cl relands - https://codereview.chromium.org/1915113005 - https://codereview.chromium.org/2066763002 except for code related to Field-Trial or SharedBuffer::unlock(). BUG=603462 ========== to ========== [WeakMemoryCache] Make references from MemoryCache to Resource Weak Design doc (Step 1): https://docs.google.com/document/d/1Pj4vzxzCKC_11SugdJ_x7wq7ErE-pHtf4w4FLKdcF... This cl relands - https://codereview.chromium.org/1915113005 - https://codereview.chromium.org/2066763002 except for code related to Field-Trial or SharedBuffer::unlock(). BUG=603462 Committed: https://crrev.com/581fd42e219de5510f0fd7729e7b00a411ff68a8 Cr-Commit-Position: refs/heads/master@{#417245} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/581fd42e219de5510f0fd7729e7b00a411ff68a8 Cr-Commit-Position: refs/heads/master@{#417245} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
