Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1324)

Issue 594763002: Revert of Expanding Type Traits to make Vector use mem ops more. (Closed)

Created:
6 years, 3 months ago by Nico
Modified:
6 years, 3 months ago
CC:
aandrey+blink_chromium.org, abarth-chromium, blink-reviews, blink-reviews-wtf_chromium.org, Mikhail, Raymond Toy
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Revert of Expanding Type Traits to make Vector use mem ops more. (patchset #4 id:120001 of https://codereview.chromium.org/581683002/) Reason for revert: Broke developer builds on os x: https://code.google.com/p/chromium/issues/detail?id=416715 Original issue's description: > Expanding Type Traits to make Vector use mem ops more. > > Vector has several code paths depending on whether the encapsulated > object can be handled with memmove/memcpu/memset or not, and those > are used by many objects. Unfortunately that requires manual hinting > in each class because the type traits used are not strong enough. > > By using type trait functions available in all modern compilers it's > possible to more intelligently select the most optimal code path. > > Preferably the code would use the new type traits in C++ 11 but > unfortunately that requires a modern c++ library. Luckily, those > libraries are mostly just thin wrappers on top of de facto-standard > compiler extension functions anyway. > > With clang this cuts away 105 KB of machine code in blink. Mostly by > no longer needing copy constructor and assignment operators in many > classes so those can be stripped from the binary. > > With gcc the changes are differently. The binary is smaller but only > by 5 KB. It's clear that many Vector methods have become more compact but > it seems gcc compensated by spending that space elsewhere (more inlining?). > > I don't have as good measurement tools for Visual Studio but there was no > huge difference in footprint. > > Performance, all this is from a noisy development computer with a > non-stable cpu clock frequency (turbo ftl...): > > In LayoutTests most changes are within noise levels. But it looks like > large tables and flexboxes might be 1-5% faster. In ParserTests > html5-full-render (macro-benchmark) became 2-3% faster. > > clang footprint numbers: > Total change: -109940 bytes > =========================== > 14 added, totalling +3397 bytes across 8 sources > 19 removed, totalling -13302 bytes across 6 sources > 8 grown, for a net change of +578 bytes (2268 bytes before, 2846 bytes after) across 6 sources > 230 shrunk, for a net change of -100613 bytes (190909 bytes before, 90296 bytes after) across 49 sources > > Biggest changes: > -84410 - Source: /home/bratell/src/chromium/src/third_party/WebKit/Source/wtf/Vector.h - (gained 784, lost 85194) > -6934 - Source: /home/bratell/src/chromium/src/third_party/WebKit/Source/wtf/Deque.h - (gained 0, lost 6934) > -2267 - Source: /home/bratell/src/chromium/src/third_party/WebKit/Source/core/animation/css/CSSAnimationData.cpp - (gained 904, lost 3171) > ... > +245 - Source: /home/bratell/src/chromium/src/third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp - (gained 245, lost 0) > > BUG= > R=jochen@chromium.org > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182427 TBR=jochen@chromium.org,thakis@chomium.org,bratell@opera.com NOTREECHECKS=true NOTRY=true BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182459

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -145 lines) Patch
M Source/wtf/TypeTraits.h View 3 chunks +5 lines, -40 lines 0 comments Download
M Source/wtf/TypeTraits.cpp View 2 chunks +1 line, -84 lines 0 comments Download
M Source/wtf/VectorTraits.h View 4 chunks +7 lines, -21 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Nico
Created Revert of Expanding Type Traits to make Vector use mem ops more.
6 years, 3 months ago (2014-09-23 01:26:32 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/594763002/1
6 years, 3 months ago (2014-09-23 01:26:44 UTC) #2
commit-bot: I haz the power
6 years, 3 months ago (2014-09-23 01:27:15 UTC) #3
Message was sent while issue was closed.
Committed patchset #1 (id:1) as 182459

Powered by Google App Engine
This is Rietveld 408576698