New issue
Advanced search Search tips

Issue 603791 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 603462

Blocking:
issue 569162



Sign in to add a comment

Deprecate PurgeableVector and SharedBuffer::unlock()

Project Member Reported by kinuko@chromium.org, Apr 15 2016

Issue description

Once we can make MemoryCache weak (issue 603462) and let Resource's get collected after all clients are removed, we should be able to remove SharedBuffer::unlock.
 

Comment 1 by kinuko@chromium.org, Apr 15 2016

Blockedon: 603462
Blocking: 569162
Project Member

Comment 2 by sheriffbot@chromium.org, Jun 1 2016

Labels: -M-52 M-53 MovedFrom-52
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
This is a follow up on - 569162 , can we have an ETA for this issue. 
I'm thinking we might be able to remove SharedBuffer::unlock() if the resurrection rate -- successful lock() after unlock() -- is sufficiently low, independently from Weak MemoryCache.
To investigate the ratio, I uploaded a CL: https://codereview.chromium.org/2140513002/

Project Member

Comment 5 by sheriffbot@chromium.org, Jul 13 2016

Labels: -M-53 MovedFrom-53
This issue has been moved once and is lower than Pri-1. Removing the milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48dcde03af18a3065e0e89b2de1e40df3e78af67

commit 48dcde03af18a3065e0e89b2de1e40df3e78af67
Author: hiroshige <hiroshige@chromium.org>
Date: Sun Jul 17 15:09:38 2016

Add UMAs for SharedBuffer::lock()/unlock()

If SharedBuffer is rarely lock()ed successfully after its unlock(), we can
remove SharedBuffer::unlock() without significant performance regressions.
To evaluate this assumption, this CL adds UMAs for SharedBuffer::lock() and
unlock() counts.

This CL also fixes ResourceType enum names in histograms.xml.
ResourceType enum is introduced in 2013 and left unmodified since then, but
it is now inconsistent with C++'s Resource::Type values.
Because ResourceType enum has been unused recently (it is referenced in
histograms.xml only from WebCore.ResourceFetcher.ResourceTypeUponCacheHit,
which doesn't have C++ counterpart nor UMA reports at least since 2016/01),
updating the ResourceType enum values in histograms.xml looks safe.

BUG= 603791 

Review-Url: https://codereview.chromium.org/2140513002
Cr-Commit-Position: refs/heads/master@{#405956}

[modify] https://crrev.com/48dcde03af18a3065e0e89b2de1e40df3e78af67/third_party/WebKit/Source/core/fetch/Resource.cpp
[modify] https://crrev.com/48dcde03af18a3065e0e89b2de1e40df3e78af67/third_party/WebKit/Source/core/fetch/Resource.h
[modify] https://crrev.com/48dcde03af18a3065e0e89b2de1e40df3e78af67/tools/metrics/histograms/histograms.xml

Status: Assigned (was: Available)
Status: Started (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57

commit 18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57
Author: hiroshige <hiroshige@chromium.org>
Date: Wed Aug 24 07:10:44 2016

Remove SharedBuffer::unlock() and keep Resource's SharedBuffer always locked

This CL makes Resource's SharedBuffer always locked and never unlocked by
removing SharedBuffer::unlock(), and
propagates the change by removing the following:
- Resource::lock(), unlock() (does nothing and returns always true)
- Resource::isPurgeable() (always false)
- Resource::isSafeToUnlock() (no longer called)
- SharedBuffer::lock(), unlock(), isLocked() (always true)
- PurgeableVector::lock(), unlock(), isLocked() (always true)
- Unit tests related to SharedBuffer/PurgeableVector::lock()/unlock() from
  PurgeableVectorTest and ResourceTest.

After this CL, the SharedBuffer is still allocated as discardable memory.
The SharedBuffer will be turned into non-discardable by
https://codereview.chromium.org/2247073007/.

BUG= 603791 , 569162

Review-Url: https://codereview.chromium.org/2253853002
Cr-Commit-Position: refs/heads/master@{#414005}

[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.cpp
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.h
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/fetch/FontResource.cpp
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/fetch/FontResource.h
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/fetch/ImageResource.cpp
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/fetch/ImageResource.h
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/fetch/MemoryCache.cpp
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/fetch/Resource.cpp
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/fetch/Resource.h
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/fetch/ResourceTest.cpp
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/fetch/ScriptResource.cpp
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/platform/PurgeableVector.cpp
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/platform/PurgeableVector.h
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/platform/PurgeableVectorTest.cpp
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/platform/SharedBuffer.cpp
[modify] https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57/third_party/WebKit/Source/platform/SharedBuffer.h

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 31 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8fd341495e433ab7042c60cf0f099551ef249e7b

commit 8fd341495e433ab7042c60cf0f099551ef249e7b
Author: hiroshige <hiroshige@chromium.org>
Date: Wed Aug 31 05:37:28 2016

Make SharedBuffer in Resource non-discardable and remove PurgeableVector

This CL makes SharedBuffer in Resource non-discardable (i.e. replaces
SharedBuffer::createPurgeable() with SharedBuffer::create()), as we no longer
have SharedBuffer::lock()/unlock() after
https://codereview.chromium.org/2253853002/.

After that, all SharedBuffers are non-purgeable, and thus this CL makes
SharedBuffer always non-purgeable by replacing PurgeableVector in SharedBuffer
with WTF::Vector.

BUG= 603791 

Review-Url: https://codereview.chromium.org/2247073007
Cr-Commit-Position: refs/heads/master@{#415568}

[modify] https://crrev.com/8fd341495e433ab7042c60cf0f099551ef249e7b/third_party/WebKit/Source/core/fetch/Resource.cpp
[delete] https://crrev.com/6fcceab2ad6b25e6015097ab6033ec4977f06387/third_party/WebKit/Source/platform/PurgeableVector.cpp
[delete] https://crrev.com/6fcceab2ad6b25e6015097ab6033ec4977f06387/third_party/WebKit/Source/platform/PurgeableVector.h
[delete] https://crrev.com/6fcceab2ad6b25e6015097ab6033ec4977f06387/third_party/WebKit/Source/platform/PurgeableVectorTest.cpp
[modify] https://crrev.com/8fd341495e433ab7042c60cf0f099551ef249e7b/third_party/WebKit/Source/platform/SharedBuffer.cpp
[modify] https://crrev.com/8fd341495e433ab7042c60cf0f099551ef249e7b/third_party/WebKit/Source/platform/SharedBuffer.h
[modify] https://crrev.com/8fd341495e433ab7042c60cf0f099551ef249e7b/third_party/WebKit/Source/platform/SharedBufferTest.cpp
[modify] https://crrev.com/8fd341495e433ab7042c60cf0f099551ef249e7b/third_party/WebKit/Source/platform/blink_platform.gypi

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 31 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/022d502a9aec8930029c9c3b05eedb4144308f14

commit 022d502a9aec8930029c9c3b05eedb4144308f14
Author: hiroshige <hiroshige@chromium.org>
Date: Wed Aug 31 10:23:11 2016

Remove purgedSize and purgeableSize from TypeStatistic and ResourceTypeStat

They are unused after https://codereview.chromium.org/2253853002/ and thus this
CL removes them from MemoryCache::TypeStatistic and WebCache::ResourceTypeStat.

This CL also fixes compile errors when MEMORY_CACHE_STATS is enabled.

BUG= 603791 

Review-Url: https://codereview.chromium.org/2254593003
Cr-Commit-Position: refs/heads/master@{#415602}

[modify] https://crrev.com/022d502a9aec8930029c9c3b05eedb4144308f14/third_party/WebKit/Source/core/fetch/MemoryCache.cpp
[modify] https://crrev.com/022d502a9aec8930029c9c3b05eedb4144308f14/third_party/WebKit/Source/core/fetch/MemoryCache.h
[modify] https://crrev.com/022d502a9aec8930029c9c3b05eedb4144308f14/third_party/WebKit/Source/web/WebCache.cpp
[modify] https://crrev.com/022d502a9aec8930029c9c3b05eedb4144308f14/third_party/WebKit/public/web/WebCache.h

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/de40c6d2ae75ce62ee8fbf11066dacedb016e987

commit de40c6d2ae75ce62ee8fbf11066dacedb016e987
Author: hiroshige <hiroshige@chromium.org>
Date: Fri Sep 02 12:02:54 2016

MemoryCache: Remove MEMORY_CACHE_STATS

Now that we have memory-infra, I expect we can remove debug code under
MEMORY_CACHE_STATS.

This CL removes printf() debug code that cause presubmit warnings
(follow-up of a comment in https://codereview.chromium.org/2254593003/).

BUG= 603791 

Review-Url: https://codereview.chromium.org/2299603002
Cr-Commit-Position: refs/heads/master@{#416242}

[modify] https://crrev.com/de40c6d2ae75ce62ee8fbf11066dacedb016e987/third_party/WebKit/Source/core/fetch/MemoryCache.cpp
[modify] https://crrev.com/de40c6d2ae75ce62ee8fbf11066dacedb016e987/third_party/WebKit/Source/core/fetch/MemoryCache.h

Status: Fixed (was: Started)
Closing as fixed. This issue removed ~1000 lines in total!

Sign in to add a comment