PartitionAlloc: Centralize Partition allocators into one place (Part 1) (issue 1041103002 by haraken@chromium.org)

0 views
Skip to first unread message

har...@chromium.org

unread,
Mar 30, 2015, 1:47:54 AM3/30/15
to oilpan-...@chromium.org, cev...@chromium.org, tse...@chromium.org, blink-...@chromium.org, vive...@samsung.com, dongseo...@intel.com, eae+bli...@chromium.org, leviw+b...@chromium.org, viv...@chromium.org, blink-re...@chromium.org, aandre...@chromium.org, rob....@samsung.com, arv+...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, blink-revie...@chromium.org, jchaffraix...@chromium.org, blink-rev...@chromium.org, ag...@chromium.org, zol...@webkit.org, sigb...@opera.com, dominik....@intel.com, blink-revie...@chromium.org, pdr+renderi...@chromium.org, leviw+re...@chromium.org, slimming-pa...@chromium.org, oilpan-...@chromium.org, kouhe...@chromium.org, mikhail.p...@intel.com
Reviewers: oilpan-reviews, Chris Evans, Tom Sepez,

Message:
PTAL

In essence, this CL is a trivial change. Just merged platform/Partitions.h
into
wtf/WTF.h, and renamed wtf/WTF.h to wtf/Partitions.h.


Description:
PartitionAlloc: Centralize Partition allocators into one place (Part 1)

Currently partitions allocators are scattered in multiple places in the code
base
(i.e., wtf/WTF.h, wtf/FastMalloc.h, platform/Partitions.h) and it's hard
to collect PartitionAlloc-wide profiling data (e.g., how much memory is
consumed
in
PartitionAlloc).

As a first step, this CL merges platform/Partitions.h into wtf/WTF.h.
Also this CL renames wtf/WTF.h to wtf/Partitions.h.

BUG=471650

Please review this at https://codereview.chromium.org/1041103002/

Base URL: svn://svn.chromium.org/blink/trunk

Affected files (+51, -305 lines):
M Source/bindings/core/v8/V8GCController.cpp
M Source/core/Init.cpp
M Source/core/dom/Node.cpp
M Source/core/layout/LayoutObject.cpp
M Source/core/layout/LayoutState.cpp
M Source/core/layout/PaintInvalidationState.cpp
M Source/core/layout/line/InlineBox.cpp
M Source/core/paint/DeprecatedPaintLayer.cpp
D Source/platform/Partitions.h
D Source/platform/Partitions.cpp
M Source/platform/blink_platform.gypi
M Source/platform/heap/RunAllTests.cpp
M Source/platform/testing/RunAllTests.cpp
M Source/platform/text/BidiCharacterRun.cpp
M Source/web/WebKit.cpp
M Source/wtf/ArrayBufferContents.cpp
M Source/wtf/BitVector.cpp
M Source/wtf/DefaultAllocator.h
M Source/wtf/HashTable.h
A + Source/wtf/Partitions.h
A + Source/wtf/Partitions.cpp
M Source/wtf/ThreadSpecific.h
M Source/wtf/Vector.h
D Source/wtf/WTF.h
D Source/wtf/WTF.cpp
M Source/wtf/testing/RunAllTests.cpp
M Source/wtf/text/CString.cpp
M Source/wtf/text/StringImpl.cpp
M Source/wtf/wtf.gypi


cev...@chromium.org

unread,
Mar 30, 2015, 2:41:37 PM3/30/15
to har...@chromium.org, oilpan-...@chromium.org, tse...@chromium.org, blink-...@chromium.org, vive...@samsung.com, dongseo...@intel.com, eae+bli...@chromium.org, leviw+b...@chromium.org, viv...@chromium.org, blink-re...@chromium.org, aandre...@chromium.org, rob....@samsung.com, arv+...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, blink-revie...@chromium.org, jchaffraix...@chromium.org, blink-rev...@chromium.org, ag...@chromium.org, zol...@webkit.org, sigb...@opera.com, dominik....@intel.com, har...@chromium.org, blink-revie...@chromium.org, pdr+renderi...@chromium.org, leviw+re...@chromium.org, slimming-pa...@chromium.org, oilpan-...@chromium.org, kouhe...@chromium.org, mikhail.p...@intel.com
LGTM


https://codereview.chromium.org/1041103002/diff/1/Source/wtf/Partitions.cpp
File Source/wtf/Partitions.cpp (right):

https://codereview.chromium.org/1041103002/diff/1/Source/wtf/Partitions.cpp#newcode98
Source/wtf/Partitions.cpp:98: m_bufferAllocator.shutdown();
Nit: we're also ignoring a fairly useful return code from this third
shutdown, so I recommend making it explicit that this is deliberate too
with another (void) prefix.

https://codereview.chromium.org/1041103002/diff/1/Source/wtf/Partitions.h
File Source/wtf/Partitions.h (right):

https://codereview.chromium.org/1041103002/diff/1/Source/wtf/Partitions.h#newcode57
Source/wtf/Partitions.h:57: ALWAYS_INLINE static PartitionRoot*
getObjectModelPartition() { return m_objectModelAllocator.root(); }
Nit: "ALWAYS_INLINE static" is a different ordering to "static
ALWAYS_INLINE". I'm not sure which is correct, but they should be the
same :-)

https://codereview.chromium.org/1041103002/

tse...@chromium.org

unread,
Mar 30, 2015, 2:53:12 PM3/30/15
to har...@chromium.org, oilpan-...@chromium.org, cev...@chromium.org, blink-...@chromium.org, vive...@samsung.com, dongseo...@intel.com, eae+bli...@chromium.org, leviw+b...@chromium.org, viv...@chromium.org, blink-re...@chromium.org, aandre...@chromium.org, rob....@samsung.com, arv+...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, blink-revie...@chromium.org, jchaffraix...@chromium.org, blink-rev...@chromium.org, ag...@chromium.org, zol...@webkit.org, sigb...@opera.com, dominik....@intel.com, har...@chromium.org, blink-revie...@chromium.org, pdr+renderi...@chromium.org, leviw+re...@chromium.org, slimming-pa...@chromium.org, oilpan-...@chromium.org, kouhe...@chromium.org, mikhail.p...@intel.com
lgtm



https://codereview.chromium.org/1041103002/

har...@chromium.org

unread,
Mar 30, 2015, 7:06:18 PM3/30/15
to oilpan-...@chromium.org, cev...@chromium.org, tse...@chromium.org, blink-...@chromium.org, vive...@samsung.com, dongseo...@intel.com, eae+bli...@chromium.org, leviw+b...@chromium.org, viv...@chromium.org, blink-re...@chromium.org, aandre...@chromium.org, rob....@samsung.com, arv+...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, blink-revie...@chromium.org, jchaffraix...@chromium.org, blink-rev...@chromium.org, ag...@chromium.org, zol...@webkit.org, sigb...@opera.com, dominik....@intel.com, blink-revie...@chromium.org, pdr+renderi...@chromium.org, leviw+re...@chromium.org, slimming-pa...@chromium.org, oilpan-...@chromium.org, kouhe...@chromium.org, mikhail.p...@intel.com
Thanks for review.



https://codereview.chromium.org/1041103002/diff/1/Source/wtf/Partitions.cpp
File Source/wtf/Partitions.cpp (right):

https://codereview.chromium.org/1041103002/diff/1/Source/wtf/Partitions.cpp#newcode98
Source/wtf/Partitions.cpp:98: m_bufferAllocator.shutdown();
On 2015/03/30 18:41:36, Chris Evans wrote:
> Nit: we're also ignoring a fairly useful return code from this third
shutdown,
> so I recommend making it explicit that this is deliberate too with
another
> (void) prefix.

Done.

https://codereview.chromium.org/1041103002/diff/1/Source/wtf/Partitions.h
File Source/wtf/Partitions.h (right):

https://codereview.chromium.org/1041103002/diff/1/Source/wtf/Partitions.h#newcode57
Source/wtf/Partitions.h:57: ALWAYS_INLINE static PartitionRoot*
getObjectModelPartition() { return m_objectModelAllocator.root(); }
On 2015/03/30 18:41:36, Chris Evans wrote:
> Nit: "ALWAYS_INLINE static" is a different ordering to "static
ALWAYS_INLINE".
> I'm not sure which is correct, but they should be the same :-)

Blink mixes both, but it seems 'ALWAYS_INLINE static' is more popular.
Used 'ALWAYS_INLINE static' consistently in this file.

https://codereview.chromium.org/1041103002/

commi...@chromium.org

unread,
Mar 30, 2015, 7:06:36 PM3/30/15
to har...@chromium.org, oilpan-...@chromium.org, cev...@chromium.org, tse...@chromium.org, blink-...@chromium.org, vive...@samsung.com, dongseo...@intel.com, eae+bli...@chromium.org, leviw+b...@chromium.org, viv...@chromium.org, blink-re...@chromium.org, aandre...@chromium.org, rob....@samsung.com, arv+...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, blink-revie...@chromium.org, jchaffraix...@chromium.org, blink-rev...@chromium.org, ag...@chromium.org, zol...@webkit.org, sigb...@opera.com, dominik....@intel.com, har...@chromium.org, blink-revie...@chromium.org, pdr+renderi...@chromium.org, leviw+re...@chromium.org, slimming-pa...@chromium.org, oilpan-...@chromium.org, kouhe...@chromium.org, mikhail.p...@intel.com
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1041103002/20001


https://codereview.chromium.org/1041103002/

commi...@chromium.org

unread,
Mar 30, 2015, 7:38:04 PM3/30/15
to har...@chromium.org, oilpan-...@chromium.org, cev...@chromium.org, tse...@chromium.org, blink-...@chromium.org, vive...@samsung.com, dongseo...@intel.com, eae+bli...@chromium.org, leviw+b...@chromium.org, viv...@chromium.org, blink-re...@chromium.org, aandre...@chromium.org, rob....@samsung.com, arv+...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, blink-revie...@chromium.org, jchaffraix...@chromium.org, blink-rev...@chromium.org, ag...@chromium.org, zol...@webkit.org, sigb...@opera.com, dominik....@intel.com, har...@chromium.org, blink-revie...@chromium.org, pdr+renderi...@chromium.org, leviw+re...@chromium.org, slimming-pa...@chromium.org, oilpan-...@chromium.org, kouhe...@chromium.org, mikhail.p...@intel.com
Try jobs failed on following builders:
android_chromium_gn_compile_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_compile_rel/builds/31098)

https://codereview.chromium.org/1041103002/

commi...@chromium.org

unread,
Mar 30, 2015, 7:55:31 PM3/30/15
to har...@chromium.org, oilpan-...@chromium.org, cev...@chromium.org, tse...@chromium.org, blink-...@chromium.org, vive...@samsung.com, dongseo...@intel.com, eae+bli...@chromium.org, leviw+b...@chromium.org, viv...@chromium.org, blink-re...@chromium.org, aandre...@chromium.org, rob....@samsung.com, arv+...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, blink-revie...@chromium.org, jchaffraix...@chromium.org, blink-rev...@chromium.org, ag...@chromium.org, zol...@webkit.org, sigb...@opera.com, dominik....@intel.com, har...@chromium.org, blink-revie...@chromium.org, pdr+renderi...@chromium.org, leviw+re...@chromium.org, slimming-pa...@chromium.org, oilpan-...@chromium.org, kouhe...@chromium.org, mikhail.p...@intel.com
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1041103002/40001


https://codereview.chromium.org/1041103002/

commi...@chromium.org

unread,
Mar 30, 2015, 9:10:09 PM3/30/15
to har...@chromium.org, oilpan-...@chromium.org, cev...@chromium.org, tse...@chromium.org, blink-...@chromium.org, vive...@samsung.com, dongseo...@intel.com, eae+bli...@chromium.org, leviw+b...@chromium.org, viv...@chromium.org, blink-re...@chromium.org, aandre...@chromium.org, rob....@samsung.com, arv+...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, blink-revie...@chromium.org, jchaffraix...@chromium.org, blink-rev...@chromium.org, ag...@chromium.org, zol...@webkit.org, sigb...@opera.com, dominik....@intel.com, har...@chromium.org, blink-revie...@chromium.org, pdr+renderi...@chromium.org, leviw+re...@chromium.org, slimming-pa...@chromium.org, oilpan-...@chromium.org, kouhe...@chromium.org, mikhail.p...@intel.com
Try jobs failed on following builders:
linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/54946)

https://codereview.chromium.org/1041103002/

har...@chromium.org

unread,
Mar 31, 2015, 1:34:22 AM3/31/15
to oilpan-...@chromium.org, cev...@chromium.org, tse...@chromium.org, blink-...@chromium.org, vive...@samsung.com, dongseo...@intel.com, eae+bli...@chromium.org, leviw+b...@chromium.org, viv...@chromium.org, blink-re...@chromium.org, aandre...@chromium.org, rob....@samsung.com, arv+...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, blink-revie...@chromium.org, jchaffraix...@chromium.org, blink-rev...@chromium.org, ag...@chromium.org, zol...@webkit.org, sigb...@opera.com, dominik....@intel.com, blink-revie...@chromium.org, pdr+renderi...@chromium.org, leviw+re...@chromium.org, slimming-pa...@chromium.org, oilpan-...@chromium.org, kouhe...@chromium.org, mikhail.p...@intel.com
Fixed broken tests. Landing.


https://codereview.chromium.org/1041103002/

commi...@chromium.org

unread,
Mar 31, 2015, 1:34:39 AM3/31/15
to har...@chromium.org, oilpan-...@chromium.org, cev...@chromium.org, tse...@chromium.org, blink-...@chromium.org, vive...@samsung.com, dongseo...@intel.com, eae+bli...@chromium.org, leviw+b...@chromium.org, viv...@chromium.org, blink-re...@chromium.org, aandre...@chromium.org, rob....@samsung.com, arv+...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, blink-revie...@chromium.org, jchaffraix...@chromium.org, blink-rev...@chromium.org, ag...@chromium.org, zol...@webkit.org, sigb...@opera.com, dominik....@intel.com, blink-revie...@chromium.org, pdr+renderi...@chromium.org, leviw+re...@chromium.org, slimming-pa...@chromium.org, oilpan-...@chromium.org, kouhe...@chromium.org, mikhail.p...@intel.com
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1041103002/100001


https://codereview.chromium.org/1041103002/

commi...@chromium.org

unread,
Mar 31, 2015, 3:07:54 AM3/31/15
to har...@chromium.org, oilpan-...@chromium.org, cev...@chromium.org, tse...@chromium.org, blink-...@chromium.org, vive...@samsung.com, dongseo...@intel.com, eae+bli...@chromium.org, leviw+b...@chromium.org, viv...@chromium.org, blink-re...@chromium.org, aandre...@chromium.org, rob....@samsung.com, arv+...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, blink-revie...@chromium.org, jchaffraix...@chromium.org, blink-rev...@chromium.org, ag...@chromium.org, zol...@webkit.org, sigb...@opera.com, dominik....@intel.com, blink-revie...@chromium.org, pdr+renderi...@chromium.org, leviw+re...@chromium.org, slimming-pa...@chromium.org, oilpan-...@chromium.org, kouhe...@chromium.org, mikhail.p...@intel.com
Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192809

https://codereview.chromium.org/1041103002/
Reply all
Reply to author
Forward
0 new messages