Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1904923002:
Reduce a shadow root object's size by not inheriting DoublyLinkedList (Closed)
|
Created:
4 years, 8 months ago by hayato Modified:
4 years, 8 months ago CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, rwlbuis, sof, webcomponents-bugzilla_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@raredatav0 Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReduce a shadow root object's size by not inheriting DoublyLinkedList
Using DoublyLinkedList increases a shadow root object's size by adding two pointers,
m_prev and m_next.
Because these pointers are only required for multiple shadow roots,
these pointers can be moved to ShadowRootRareDataV0.
- Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList.
- ElementShadow has only one shadowRoot object, instead of having DoublyLinkedList<ShadowRoot>.
- ElementShadow::m_shadowRoot acts as the oldest shadow root in case for multiple shadow roots.
BUG=531990
Committed: https://crrev.com/c47ae70a879d7247c56a842716f4ab67ac25a71d
Cr-Commit-Position: refs/heads/master@{#389013}
Patch Set 1 #
Total comments: 6
Patch Set 2 : use DCHECK_EQ #Patch Set 3 : fix typo #
Messages
Total messages: 42 (23 generated)
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1904923002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1904923002/1
Description was changed from ========== No longer use a doubly linked list to reduce the footprint of a shadow root object BUG=531990 ========== to ========== Reduce the footprint of a shadow root object by not inheriting DoublyLinkedList Using DoublyLinkedList increases the memory size of a ShadowRoot object by the size of two pointers, prev and next. Because these pointers are only required for multiple shadow roots, two pointers are moved to ShadowRootRareDataV0 and maintain doubly linked list manually. BUG=531990 ==========
Description was changed from ========== Reduce the footprint of a shadow root object by not inheriting DoublyLinkedList Using DoublyLinkedList increases the memory size of a ShadowRoot object by the size of two pointers, prev and next. Because these pointers are only required for multiple shadow roots, two pointers are moved to ShadowRootRareDataV0 and maintain doubly linked list manually. BUG=531990 ========== to ========== Reduce the footprint of a shadow root object by not inheriting DoublyLinkedList Using DoublyLinkedList increases the memory size of a ShadowRoot object by the size of two pointers, prev and next. Because these pointers are only required for multiple shadow roots, these pointers are moved to ShadowRootRareDataV0 and maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. BUG=531990 ==========
Description was changed from ========== Reduce the footprint of a shadow root object by not inheriting DoublyLinkedList Using DoublyLinkedList increases the memory size of a ShadowRoot object by the size of two pointers, prev and next. Because these pointers are only required for multiple shadow roots, these pointers are moved to ShadowRootRareDataV0 and maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. BUG=531990 ========== to ========== Reduce the footprint of a shadow root object by not inheriting DoublyLinkedList Using DoublyLinkedList increases the memory size of a ShadowRoot object by the size of two pointers, prev and next. Because these pointers are only required for multiple shadow roots, these pointers can be moved to ShadowRootRareDataV0. Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. BUG=531990 ==========
Description was changed from ========== Reduce the footprint of a shadow root object by not inheriting DoublyLinkedList Using DoublyLinkedList increases the memory size of a ShadowRoot object by the size of two pointers, prev and next. Because these pointers are only required for multiple shadow roots, these pointers can be moved to ShadowRootRareDataV0. Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. BUG=531990 ========== to ========== Reduce a shadow root object's size by not inheriting DoublyLinkedList Using DoublyLinkedList increases a shadow root object's size by adding two pointers, prev and next. Because these pointers are only required for multiple shadow roots, these pointers can be moved to ShadowRootRareDataV0. Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. BUG=531990 ==========
Description was changed from ========== Reduce a shadow root object's size by not inheriting DoublyLinkedList Using DoublyLinkedList increases a shadow root object's size by adding two pointers, prev and next. Because these pointers are only required for multiple shadow roots, these pointers can be moved to ShadowRootRareDataV0. Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. BUG=531990 ========== to ========== Reduce a shadow root object's size by not inheriting DoublyLinkedList Using DoublyLinkedList increases a shadow root object's size by adding two pointers, m_prev and m_next. Because these pointers are only required for multiple shadow roots, these pointers can be moved to ShadowRootRareDataV0. - Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. - Now ElementShadow has only one shadowRoot object, which is the oldest shadow root in case for multiple shadow roots. BUG=531990 ==========
Description was changed from ========== Reduce a shadow root object's size by not inheriting DoublyLinkedList Using DoublyLinkedList increases a shadow root object's size by adding two pointers, m_prev and m_next. Because these pointers are only required for multiple shadow roots, these pointers can be moved to ShadowRootRareDataV0. - Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. - Now ElementShadow has only one shadowRoot object, which is the oldest shadow root in case for multiple shadow roots. BUG=531990 ========== to ========== Reduce a shadow root object's size by not inheriting DoublyLinkedList Using DoublyLinkedList increases a shadow root object's size by adding two pointers, m_prev and m_next. Because these pointers are only required for multiple shadow roots, these pointers can be moved to ShadowRootRareDataV0. - Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. - Now ElementShadow has only one shadowRoot object, which is the oldest shadow root in case for multiple shadow roots. BUG=531990 ==========
hayato@chromium.org changed reviewers: + kochi@chromium.org, tkent@chromium.org
PTAL
Description was changed from ========== Reduce a shadow root object's size by not inheriting DoublyLinkedList Using DoublyLinkedList increases a shadow root object's size by adding two pointers, m_prev and m_next. Because these pointers are only required for multiple shadow roots, these pointers can be moved to ShadowRootRareDataV0. - Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. - Now ElementShadow has only one shadowRoot object, which is the oldest shadow root in case for multiple shadow roots. BUG=531990 ========== to ========== Reduce a shadow root object's size by not inheriting DoublyLinkedList Using DoublyLinkedList increases a shadow root object's size by adding two pointers, m_prev and m_next. Because these pointers are only required for multiple shadow roots, these pointers can be moved to ShadowRootRareDataV0. - Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. - ElementShadow has only one shadowRoot object, instead of having DoublyLinkedList<ShadowRoot>. - ElementShadow::m_shadowRoot acts as the oldest shadow root in case for multiple shadow roots. BUG=531990 ==========
lgtm https://codereview.chromium.org/1904923002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp (right): https://codereview.chromium.org/1904923002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp:162: DCHECK(m_shadowRoot->type() == ShadowRootType::V0); DCHECK -> DCHECK_EQ https://codereview.chromium.org/1904923002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp (right): https://codereview.chromium.org/1904923002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp:95: DCHECK(type() == ShadowRootType::V0); DCHCECK -> DCHECK_EQ https://codereview.chromium.org/1904923002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp:101: DCHECK(type() == ShadowRootType::V0); DCHECK -> DCHECK_EQ
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
use DCHECK_EQ
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1904923002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1904923002/20001
Ops. I forgot that you added "operater<<" for ShadowRootType. Nice. https://codereview.chromium.org/1904923002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp (right): https://codereview.chromium.org/1904923002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp:162: DCHECK(m_shadowRoot->type() == ShadowRootType::V0); Done https://codereview.chromium.org/1904923002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp (right): https://codereview.chromium.org/1904923002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp:95: DCHECK(type() == ShadowRootType::V0); Done https://codereview.chromium.org/1904923002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp:101: DCHECK(type() == ShadowRootType::V0); Done
The CQ bit was unchecked by hayato@chromium.org
The CQ bit was checked by hayato@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org Link to the patchset: https://codereview.chromium.org/1904923002/#ps20001 (title: "use DCHECK_EQ")
The CQ bit was unchecked by commit-bot@chromium.org
This CL has an open dependency (Issue 1904703002 Patch 20001). Please resolve the dependency and try again. If you are sure that there is no real dependency, please use one of the options listed in https://goo.gl/9Es4OR to land the CL.
The CQ bit was checked by hayato@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1904923002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1904923002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
fix typo
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1904923002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1904923002/40001
The CQ bit was unchecked by hayato@chromium.org
The CQ bit was checked by hayato@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org Link to the patchset: https://codereview.chromium.org/1904923002/#ps40001 (title: "fix typo")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1904923002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1904923002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by hayato@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1904923002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1904923002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
lgtm BTW, ShadowRoot/ElementShadow was the only user of wtf/DoublyLinkedList, so if the usage is gone with this CL, can we remove DoublyLinkedList implementation?
Message was sent while issue was closed.
> BTW, ShadowRoot/ElementShadow was the only user of > wtf/DoublyLinkedList, so if the usage is gone with this CL, > can we remove DoublyLinkedList implementation? I think DoublyLinkedList is used by wtf/ThreadSpecificWin.cpp and platform/graphics/ImageDecodingStore.*.
Message was sent while issue was closed.
On 2016/04/22 05:18:06, tkent wrote: > > BTW, ShadowRoot/ElementShadow was the only user of > > wtf/DoublyLinkedList, so if the usage is gone with this CL, > > can we remove DoublyLinkedList implementation? > > I think DoublyLinkedList is used by wtf/ThreadSpecificWin.cpp and > platform/graphics/ImageDecodingStore.*. Oops, I only checked core/* :(
Message was sent while issue was closed.
Description was changed from ========== Reduce a shadow root object's size by not inheriting DoublyLinkedList Using DoublyLinkedList increases a shadow root object's size by adding two pointers, m_prev and m_next. Because these pointers are only required for multiple shadow roots, these pointers can be moved to ShadowRootRareDataV0. - Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. - ElementShadow has only one shadowRoot object, instead of having DoublyLinkedList<ShadowRoot>. - ElementShadow::m_shadowRoot acts as the oldest shadow root in case for multiple shadow roots. BUG=531990 ========== to ========== Reduce a shadow root object's size by not inheriting DoublyLinkedList Using DoublyLinkedList increases a shadow root object's size by adding two pointers, m_prev and m_next. Because these pointers are only required for multiple shadow roots, these pointers can be moved to ShadowRootRareDataV0. - Maintain a doubly linked list manually, instead of inheriting DoublyLinkedList. - ElementShadow has only one shadowRoot object, instead of having DoublyLinkedList<ShadowRoot>. - ElementShadow::m_shadowRoot acts as the oldest shadow root in case for multiple shadow roots. BUG=531990 Committed: https://crrev.com/c47ae70a879d7247c56a842716f4ab67ac25a71d Cr-Commit-Position: refs/heads/master@{#389013} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/c47ae70a879d7247c56a842716f4ab67ac25a71d Cr-Commit-Position: refs/heads/master@{#389013} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
