Refactor PartitionAlloc API |
|
Issue descriptionThe PartitionAlloc set of APIs are written in a mix of old-skool C-style "objects" where you have free functions taking a "struct" as a parameter, and you do pointer math with sizeof() to downcast when faking inheritance. The implicit "namespacing" that happens with methods is done via manual name mangling (eg, PartitionDumperStats/PartitionDumperStatsGeneric). This is also mixed into the "public" API. All this makes it hard to sort out which functions work on which data. This bug is to track refactoring of the free functions into class methods and moving of constants + fields into private sections of the classes. Ultimately, this should clarify what is the public API surface of partition alloc and lower the amount of mental coupling between variables/constants when reading the code. Given the size of the codebase, I'm gonna approach this with incremental (nearly mechanical) refactors. This might cause some churn, but without a bit of initial refactoring, it's a little difficult to see where the correct API boundaries should be....
,
Nov 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6c23ef814e6cf77ce842cf66f984d7f9930da94f commit 6c23ef814e6cf77ce842cf66f984d7f9930da94f Author: Albert J. Wong <ajwong@chromium.org> Date: Tue Nov 21 03:25:36 2017 PartitionAlloc: Make all PartitionRoot free funcs methods This is a mechanical refactor moving all free functions that take PartitionRoot as the first parameter into the PartitionRoot struct. See related bug for more details on methodology. Bug: 787153 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I94d1da2a7995e9dfdbfa9882e719107c55d9f72e Reviewed-on: https://chromium-review.googlesource.com/780447 Commit-Queue: Albert J. Wong <ajwong@chromium.org> Reviewed-by: Chris Palmer <palmer@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#518077} [modify] https://crrev.com/6c23ef814e6cf77ce842cf66f984d7f9930da94f/base/allocator/partition_allocator/partition_alloc.cc [modify] https://crrev.com/6c23ef814e6cf77ce842cf66f984d7f9930da94f/base/allocator/partition_allocator/partition_alloc.h [modify] https://crrev.com/6c23ef814e6cf77ce842cf66f984d7f9930da94f/base/allocator/partition_allocator/partition_alloc_unittest.cc [modify] https://crrev.com/6c23ef814e6cf77ce842cf66f984d7f9930da94f/third_party/WebKit/Source/core/layout/LayoutObject.cpp [modify] https://crrev.com/6c23ef814e6cf77ce842cf66f984d7f9930da94f/third_party/WebKit/Source/core/layout/line/InlineBox.cpp [modify] https://crrev.com/6c23ef814e6cf77ce842cf66f984d7f9930da94f/third_party/WebKit/Source/core/paint/PaintLayer.cpp [modify] https://crrev.com/6c23ef814e6cf77ce842cf66f984d7f9930da94f/third_party/WebKit/Source/platform/text/BidiCharacterRun.cpp [modify] https://crrev.com/6c23ef814e6cf77ce842cf66f984d7f9930da94f/third_party/WebKit/Source/platform/wtf/allocator/Partitions.cpp [modify] https://crrev.com/6c23ef814e6cf77ce842cf66f984d7f9930da94f/third_party/WebKit/Source/platform/wtf/allocator/Partitions.h
,
Nov 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8 commit f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8 Author: Albert J. Wong <ajwong@chromium.org> Date: Tue Nov 21 23:25:24 2017 PA: PartitionRootGeneric funcs -> methods This is a mechanical refactor moving all free functions that take PartitionRootGeneric as the first parameter into the PartitionRootGeneric struct. See related bug for more details on methodology. Bug: 787153 Change-Id: Ia95fc30ed030397303dc1999f4fdac8a72c01c9b Reviewed-on: https://chromium-review.googlesource.com/780759 Commit-Queue: Albert J. Wong <ajwong@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#518438} [modify] https://crrev.com/f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8/base/allocator/partition_allocator/PartitionAlloc.md [modify] https://crrev.com/f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8/base/allocator/partition_allocator/partition_alloc.cc [modify] https://crrev.com/f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8/base/allocator/partition_allocator/partition_alloc.h [modify] https://crrev.com/f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8/base/allocator/partition_allocator/partition_alloc_unittest.cc [modify] https://crrev.com/f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8/chrome/browser/profiling_host/memlog_browsertest.cc [modify] https://crrev.com/f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8/chrome/browser/profiling_host/profiling_test_driver.cc [modify] https://crrev.com/f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8/third_party/WebKit/Source/platform/wtf/allocator/PartitionAllocator.h [modify] https://crrev.com/f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8/third_party/WebKit/Source/platform/wtf/allocator/Partitions.cpp [modify] https://crrev.com/f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8/third_party/WebKit/Source/platform/wtf/allocator/Partitions.h [modify] https://crrev.com/f48b1f3e72ee8a726422dbc3fb9d0d47b251a6b8/third_party/WebKit/Source/platform/wtf/typed_arrays/ArrayBufferContents.cpp
,
Nov 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3acdd835d601570cce29ac871938f92cd1a2e470 commit 3acdd835d601570cce29ac871938f92cd1a2e470 Author: Albert J. Wong <ajwong@chromium.org> Date: Wed Nov 22 22:33:04 2017 PA: PartitionBucket public funcs -> methods This is a mechanical refactor moving all free functions in partition_alloc.h that primarily operate on PartitionBucket into the PartitionBucket struct. There are also functions inside partition_alloc.cc which are effectively private methods. Those have not been moved yet though because it's unclear if the symbol hiding within the translation unit and the "inilne" tags may or may not be disrupted if the declaration is hoisted into a struct that is shared in multiple translation units. It probably won't, but might as well do it in a separate CL. See related bug for more details on methodology. Bug: 787153 Change-Id: I52d9a8d2ef9224b30abb0b2c33823e073739b096 Reviewed-on: https://chromium-review.googlesource.com/783697 Commit-Queue: Albert J. Wong <ajwong@chromium.org> Reviewed-by: Chris Palmer <palmer@chromium.org> Reviewed-by: Albert J. Wong <ajwong@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#518773} [modify] https://crrev.com/3acdd835d601570cce29ac871938f92cd1a2e470/base/allocator/partition_allocator/partition_alloc.cc [modify] https://crrev.com/3acdd835d601570cce29ac871938f92cd1a2e470/base/allocator/partition_allocator/partition_alloc.h
,
Nov 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c3d017eb59cfe91c1aae384a8339bf76e53e34a6 commit c3d017eb59cfe91c1aae384a8339bf76e53e34a6 Author: Albert J. Wong <ajwong@chromium.org> Date: Mon Nov 27 23:06:44 2017 PA: PartitionPage public funcs -> methods This is a mechanical refactor moving all free functions in partition_alloc.h that primarily operate on PartitionPage into the PartitionPage struct. There are also functions inside partition_alloc.cc which are effectively private methods. Those have not been moved yet though because it's unclear if the symbol hiding within the translation unit and the "inilne" tags may or may not be disrupted if the declaration is hoisted into a struct that is shared in multiple translation units. It probably won't, but might as well do it in a separate CL. See related bug for more details on methodology. Bug: 787153 Change-Id: I3363ad12675bf674f4397e67f9dcc26ec384018d Reviewed-on: https://chromium-review.googlesource.com/786510 Commit-Queue: Albert J. Wong <ajwong@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Chris Palmer <palmer@chromium.org> Cr-Commit-Position: refs/heads/master@{#519442} [modify] https://crrev.com/c3d017eb59cfe91c1aae384a8339bf76e53e34a6/base/allocator/partition_allocator/partition_alloc.cc [modify] https://crrev.com/c3d017eb59cfe91c1aae384a8339bf76e53e34a6/base/allocator/partition_allocator/partition_alloc.h [modify] https://crrev.com/c3d017eb59cfe91c1aae384a8339bf76e53e34a6/base/allocator/partition_allocator/partition_alloc_unittest.cc
,
Dec 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f2d639eb4214cc6a56c18a594c184fcf63c6ae21 commit f2d639eb4214cc6a56c18a594c184fcf63c6ae21 Author: Albert J. Wong <ajwong@chromium.org> Date: Mon Dec 04 23:25:39 2017 PA: PartitionBucket: move private funcs. fix names. Previous refactors focused on moving the public free functions in the headers to methods. This CL focuses on the private functions including renaming a number of the functions so they have clearer semantics. There was some splatter into PartitionPage and PartitionFreelistEntry, but the majority of changes in those classes are reserved for a followup CL. No functionality change should be introduced here. See related bug for more details on methodology. Bug: 787153 Change-Id: I4e7c8c8364ba1257d1f42c83954565dfacbd57e3 Reviewed-on: https://chromium-review.googlesource.com/791514 Commit-Queue: Albert J. Wong <ajwong@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Chris Palmer <palmer@chromium.org> Cr-Commit-Position: refs/heads/master@{#521506} [modify] https://crrev.com/f2d639eb4214cc6a56c18a594c184fcf63c6ae21/base/allocator/partition_allocator/partition_alloc.cc [modify] https://crrev.com/f2d639eb4214cc6a56c18a594c184fcf63c6ae21/base/allocator/partition_allocator/partition_alloc.h [modify] https://crrev.com/f2d639eb4214cc6a56c18a594c184fcf63c6ae21/base/allocator/partition_allocator/partition_alloc_unittest.cc
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ee86913a1b44f2b5622c255e307d455cd6aee060 commit ee86913a1b44f2b5622c255e307d455cd6aee060 Author: Albert J. Wong <ajwong@chromium.org> Date: Tue Dec 05 19:40:37 2017 PA: PartitionPage: move file static funcs up. This moves the file local query functions that were implementing an ad-hoc "package protection" style API into the PartitionPage public API. After this, I think the functions are finally organized enough that these classes can start being detangled into separate files. See related bug for more details on methodology. Bug: 787153 Change-Id: I4997fd73e1779c9ed4f9ded9aa8a4c94c0eb7d1e Reviewed-on: https://chromium-review.googlesource.com/792090 Commit-Queue: Albert J. Wong <ajwong@chromium.org> Reviewed-by: Chris Palmer <palmer@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#521788} [modify] https://crrev.com/ee86913a1b44f2b5622c255e307d455cd6aee060/base/allocator/partition_allocator/partition_alloc.cc [modify] https://crrev.com/ee86913a1b44f2b5622c255e307d455cd6aee060/base/allocator/partition_allocator/partition_alloc.h |
|
►
Sign in to add a comment |
|
Comment 1 by ajwong@chromium.org
, Nov 20 2017