Issue metadata
Sign in to add a comment
|
0.4% regression in sizes at 487048:487048 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jul 18 2017
This is my change: https://codereview.chromium.org/2963623002 It's a significant improvement of WeakPtr's performance. It should also be a code size win, Android shrinks 48 KB, but it seems on Linux the inliner gets so excited that the binary grows instead. I'm not sure how we weigh (Linux) size vs performance in cases like this?
,
Jul 18 2017
What's 0.4% in MB? Does the perf dashboard for your change show wins?
,
Jul 18 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8973725447587479472
,
Jul 18 2017
464 KiB. The perf dashboard has a feature to see all regressions/improvements in data points that have a commit in the CL range: https://chromeperf.appspot.com/group_report?rev=487048 I kicked off a bisect to see if the speedometer improvement is due to this CL.
,
Jul 18 2017
sullivan, since you're here, any chance to change perf alerts to say "464 KiB regression" instead of "0.4% regression"? The latter is dependend on chrome's current size and also makes numbers look kind of small, while 464 KiB looks kind of large.
,
Jul 19 2017
=== Auto-CCing suspected CL author a.suchit@samsung.com === Hi a.suchit@samsung.com, the bisect results pointed to your CL, please take a look at the results. === BISECT JOB RESULTS === Perf regression found with culprit Suspected Commit Author : Suchit Agrawal Commit : 0cda8c46bcac1f766d96f3610c9bf04d939d7eb0 Date : Mon Jul 17 12:39:58 2017 Subject: Create BUILD.gn files for //media/video. Bisect Details Configuration: android_webview_arm64_aosp_perf_bisect Benchmark : speedometer Metric : React-TodoMVC/React-TodoMVC Change : 4.90% | 3882.18611111 -> 3692.09305556 Revision Result N chromium@487037 3882.19 +- 154.763 6 good chromium@487047 3838.19 +- 151.828 6 good chromium@487052 3722.1 +- 136.633 14 good chromium@487054 3714.96 +- 107.573 21 good chromium@487055 3688.43 +- 87.4133 14 bad <-- chromium@487057 3699.39 +- 70.2921 9 bad chromium@487076 3692.09 +- 58.1811 6 bad To Run This Test src/tools/perf/run_benchmark -v --browser=android-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests speedometer More information on addressing performance regressions: http://g.co/ChromePerformanceRegressions Debug information about this bisect: https://chromeperf.appspot.com/buildbucket_job_status/8973725447587479472 For feedback, file a bug with component Speed>Bisection
,
Jul 20 2017
,
Jul 20 2017
Hmmm, probably it shouldn't be a public_dep, just a dep. Also paths should not include "//media/video", but just the filename now that it's a local build.gn
,
Jul 20 2017
As I remember, it is needed in public deps. I will check it again and update. AND "//media/video" is dependent on "//gpu". We can change "//gpu" dependency to "//gpu/command_buffer/client:gles2_interface". Would it help ?
,
Jul 20 2017
//content/renderer and //cc are dependent on //media/video. So it would be in public_deps only.
,
Jul 20 2017
a.suchit@samsung.com: I'm really sorry for the noise here! I bisected an improvement in the speedometer benchmark to see if it was related to Hans's change above. The bisect thought your change sped the benchmark up. But the results are really noisy, I don't think there was an actual performance change related to your CL. thakis: I filed https://github.com/catapult-project/catapult/issues/3725 about the bug titling, thanks for the feedback!
,
Jul 21 2017
,
Jul 21 2017
> 464 KiB. That's a lot :-( > The perf dashboard has a feature to see all regressions/improvements in data points that have a commit in the CL range: https://chromeperf.appspot.com/group_report?rev=487048 It doesn't look like there's anything substantial there. I'd like to see how the UMA metrics react to this though. If it doesn't seem to improve things there, I'll try to think of something else to do with it.
,
Jul 25 2017
To double check the effects of my change, I took a look at RemoteSafeBrowsingDatabaseManager::ClientRequest::OnRequestDoneWeak before and after. The code looks like this:
void RemoteSafeBrowsingDatabaseManager::ClientRequest::OnRequestDoneWeak(
const base::WeakPtr<ClientRequest>& req,
SBThreatType matched_threat_type,
const ThreatMetadata& metadata) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
if (!req)
return; // Previously canceled
req->OnRequestDone(matched_threat_type, metadata);
}
and the call to OnRequestDone() is inlined with or without my patch.
Before my change:
0000000000000000 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE>:
0: 55 push %rbp
1: 41 57 push %r15
3: 41 56 push %r14
5: 53 push %rbx
6: 50 push %rax
7: 49 89 d6 mov %rdx,%r14
a: 41 89 f7 mov %esi,%r15d
d: 48 89 fb mov %rdi,%rbx
10: e8 00 00 00 00 callq 15 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x15>
11: R_X86_64_PLT32 _ZNK4base8internal13WeakReference8is_validEv-0x4
15: 84 c0 test %al,%al
17: 0f 84 90 00 00 00 je ad <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0xad>
1d: 48 83 7b 08 00 cmpq $0x0,0x8(%rbx)
22: 0f 84 85 00 00 00 je ad <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0xad>
28: 48 89 df mov %rbx,%rdi
2b: e8 00 00 00 00 callq 30 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x30>
2c: R_X86_64_PLT32 _ZNK4base8internal13WeakReference8is_validEv-0x4
30: 31 ed xor %ebp,%ebp
32: 84 c0 test %al,%al
34: 48 0f 45 6b 08 cmovne 0x8(%rbx),%rbp
39: 48 8b 7d 00 mov 0x0(%rbp),%rdi
3d: 48 8b 07 mov (%rdi),%rax
40: 48 8d 75 10 lea 0x10(%rbp),%rsi
44: 44 89 fa mov %r15d,%edx
47: 4c 89 f1 mov %r14,%rcx
4a: ff 50 18 callq *0x18(%rax)
4d: 48 8b 1d 00 00 00 00 mov 0x0(%rip),%rbx # 54 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x54>
50: R_X86_64_PC32 .bss._ZZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest13OnRequestDoneENS_12SBThreatTypeERKNS_14ThreatMetadataEE24atomic_histogram_pointer-0x4
54: 48 85 db test %rbx,%rbx
57: 75 2b jne 84 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x84>
59: 48 8d 3d 00 00 00 00 lea 0x0(%rip),%rdi # 60 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x60>
5c: R_X86_64_PC32 .L.str-0x4
60: be e8 03 00 00 mov $0x3e8,%esi
65: ba 80 96 98 00 mov $0x989680,%edx
6a: b9 32 00 00 00 mov $0x32,%ecx
6f: 41 b8 01 00 00 00 mov $0x1,%r8d
75: e8 00 00 00 00 callq 7a <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x7a>
76: R_X86_64_PLT32 _ZN4base9Histogram14FactoryTimeGetEPKcNS_9TimeDeltaES3_ji-0x4
7a: 48 89 c3 mov %rax,%rbx
7d: 48 89 1d 00 00 00 00 mov %rbx,0x0(%rip) # 84 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x84>
80: R_X86_64_PC32 .bss._ZZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest13OnRequestDoneENS_12SBThreatTypeERKNS_14ThreatMetadataEE24atomic_histogram_pointer-0x4
84: 48 8d bd 88 00 00 00 lea 0x88(%rbp),%rdi
8b: e8 00 00 00 00 callq 90 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x90>
8c: R_X86_64_PLT32 _ZNK4base12ElapsedTimer7ElapsedEv-0x4
90: 48 89 04 24 mov %rax,(%rsp)
94: 48 89 e6 mov %rsp,%rsi
97: 48 89 df mov %rbx,%rdi
9a: e8 00 00 00 00 callq 9f <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x9f>
9b: R_X86_64_PLT32 _ZN4base13HistogramBase7AddTimeERKNS_9TimeDeltaE-0x4
9f: 48 8b 75 00 mov 0x0(%rbp),%rsi
a3: 48 8b 7d 08 mov 0x8(%rbp),%rdi
a7: 48 8b 07 mov (%rdi),%rax
aa: ff 50 08 callq *0x8(%rax)
ad: 48 83 c4 08 add $0x8,%rsp
b1: 5b pop %rbx
b2: 41 5e pop %r14
b4: 41 5f pop %r15
b6: 5d pop %rbp
b7: c3 retq
After my change:
0000000000000000 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE>:
0: 41 56 push %r14
2: 53 push %rbx
3: 50 push %rax
4: 48 89 d0 mov %rdx,%rax
7: 89 f1 mov %esi,%ecx
9: 48 8b 17 mov (%rdi),%rdx
c: 48 8b 5f 08 mov 0x8(%rdi),%rbx
10: 48 23 5a 08 and 0x8(%rdx),%rbx
14: 74 72 je 88 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x88>
16: 48 8b 3b mov (%rbx),%rdi
19: 4c 8b 07 mov (%rdi),%r8
1c: 48 8d 73 10 lea 0x10(%rbx),%rsi
20: 89 ca mov %ecx,%edx
22: 48 89 c1 mov %rax,%rcx
25: 41 ff 50 18 callq *0x18(%r8)
29: 4c 8b 35 00 00 00 00 mov 0x0(%rip),%r14 # 30 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x30>
2c: R_X86_64_PC32 .bss._ZZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest13OnRequestDoneENS_12SBThreatTypeERKNS_14ThreatMetadataEE24atomic_histogram_pointer-0x4
30: 4d 85 f6 test %r14,%r14
33: 75 2b jne 60 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x60>
35: 48 8d 3d 00 00 00 00 lea 0x0(%rip),%rdi # 3c <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x3c>
38: R_X86_64_PC32 .L.str-0x4
3c: be e8 03 00 00 mov $0x3e8,%esi
41: ba 80 96 98 00 mov $0x989680,%edx
46: b9 32 00 00 00 mov $0x32,%ecx
4b: 41 b8 01 00 00 00 mov $0x1,%r8d
51: e8 00 00 00 00 callq 56 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x56>
52: R_X86_64_PLT32 _ZN4base9Histogram14FactoryTimeGetEPKcNS_9TimeDeltaES3_ji-0x4
56: 49 89 c6 mov %rax,%r14
59: 4c 89 35 00 00 00 00 mov %r14,0x0(%rip) # 60 <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x60>
5c: R_X86_64_PC32 .bss._ZZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest13OnRequestDoneENS_12SBThreatTypeERKNS_14ThreatMetadataEE24atomic_histogram_pointer-0x4
60: 48 8d bb 88 00 00 00 lea 0x88(%rbx),%rdi
67: e8 00 00 00 00 callq 6c <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x6c>
68: R_X86_64_PLT32 _ZNK4base12ElapsedTimer7ElapsedEv-0x4
6c: 48 89 04 24 mov %rax,(%rsp)
70: 48 89 e6 mov %rsp,%rsi
73: 4c 89 f7 mov %r14,%rdi
76: e8 00 00 00 00 callq 7b <_ZN13safe_browsing33RemoteSafeBrowsingDatabaseManager13ClientRequest17OnRequestDoneWeakERKN4base7WeakPtrIS1_EENS_12SBThreatTypeERKNS_14ThreatMetadataE+0x7b>
77: R_X86_64_PLT32 _ZN4base13HistogramBase7AddTimeERKNS_9TimeDeltaE-0x4
7b: 48 8b 33 mov (%rbx),%rsi
7e: 48 8b 7b 08 mov 0x8(%rbx),%rdi
82: 48 8b 07 mov (%rdi),%rax
85: ff 50 08 callq *0x8(%rax)
88: 48 83 c4 08 add $0x8,%rsp
8c: 5b pop %rbx
8d: 41 5e pop %r14
8f: c3 retq
Here my patch saved 40 bytes, and the sequence for doing
if (!req)
return; // Previously canceled
req->OnRequestDone(matched_threat_type, metadata);
is so much nicer! Simplified:
Before:
10: e8 00 00 00 00 callq WeakReference::is_valid
15: 84 c0 test %al,%al
17: 0f 84 90 00 00 00 je end
1d: 48 83 7b 08 00 cmpq $0x0,0x8(%rbx)
22: 0f 84 85 00 00 00 je end
28: 48 89 df mov %rbx,%rdi
2b: e8 00 00 00 00 callq WeakReference::is_valid
30: 31 ed xor %ebp,%ebp
32: 84 c0 test %al,%al
34: 48 0f 45 6b 08 cmovne 0x8(%rbx),%rbp
After:
9: 48 8b 17 mov (%rdi),%rdx
c: 48 8b 5f 08 mov 0x8(%rdi),%rbx
10: 48 23 5a 08 and 0x8(%rdx),%rbx
14: 74 72 je end
Even if it doesn't show on our perf bots, the code after is objectively better.
But since the code now becomes smaller, the inliner will inline this construct more, and I don't know how to counter that.
,
Aug 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f9ccb2f250461aa61d42780170377df6c9248e3f commit f9ccb2f250461aa61d42780170377df6c9248e3f Author: Hans Wennborg <hans@chromium.org> Date: Mon Aug 07 19:44:09 2017 Revert "WeakPtr: Reset flag_ to NullFlag() in moved-from objects" This reverts commit 3fc9f5b033889b5f0224e62cea23c55dba96268e. Reason for revert: I'm backing out the WeakPtr changes. While the new code was objectively better, it didn't show on any perf bots, regressed size on Linux quite a bit, and seems to have introduced a new crasher on Android. To summarize: it seems it's not worth it. The WeakPtr revert will be done in two patches. This one, and the main one (https://chromium-review.googlesource.com/c/604088) that also needs to be merged to M61. Original change's description: > WeakPtr: Reset flag_ to NullFlag() in moved-from objects > > A moved-from object shouldn't be used, but in case it is, set flag_ to > NullFlag() instead of nullptr to avoid crashing. > > This is a speculative fix for a crash seen in the wild. > > BUG=748495 > > Change-Id: I0b26156161c860e64ceb566e0d03cf3137ff8196 > Reviewed-on: https://chromium-review.googlesource.com/585563 > Commit-Queue: Hans Wennborg <hans@chromium.org> > Reviewed-by: Nico Weber <thakis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#489536} BUG=748495, 748075 , 744747 Change-Id: I2453f329882cd911482cc9a27c0ee99f13693d03 Reviewed-on: https://chromium-review.googlesource.com/604087 Commit-Queue: Hans Wennborg <hans@chromium.org> Commit-Queue: Nico Weber <thakis@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#492385} [modify] https://crrev.com/f9ccb2f250461aa61d42780170377df6c9248e3f/base/memory/weak_ptr.cc [modify] https://crrev.com/f9ccb2f250461aa61d42780170377df6c9248e3f/base/memory/weak_ptr.h
,
Aug 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4cfa301cda2ab84a496f3c13a5e3fa2f8de94fca commit 4cfa301cda2ab84a496f3c13a5e3fa2f8de94fca Author: Hans Wennborg <hans@chromium.org> Date: Tue Aug 08 03:33:38 2017 Revert "Make base::WeakPtr::Get() fast" This reverts commit 71f161e3bd95827876f67c656168f936d88a6e3a and 3856eab55bb241fb054a878d9922e65e7b5b0931. Reason for revert: I'm backing out the WeakPtr changes. While the new code was objectively better, it didn't show on any perf bots, regressed size on Linux quite a bit because the new code was smaller and thus inlined more often, and seems to have introduced a new crasher on Android. To summarize: it seems it's not worth it. The WeakPtr revert will be done in two patches. The first part is in https://chromium-review.googlesource.com/c/604087 This second part should be merged to M61. Original change's description: > Make base::WeakPtr::Get() fast > > It was previous calling the out-of-line method WeakReference::is_valid(). > That means that code like > > if (my_weak_ptr) > my_weak_ptr->foo(); > > would do two calls to is_valid(), plus tests and branching. > > The is_valid() method showed up as one of the most frequently called non-inline > functions during browser start-up on Windows (about 1M calls). > > is_valid() was also inefficient in itself, because it had to do a null-check > flag_, as well as checking if the flag was marked valid. > > This patch removes the null-pointer check by using a sentinel Flag object instead of > a nullptr. And instead of storing a bool in the Flag, it stores a pointer-sized > bitmask 0 or ~0, so it can be AND'ed with the pointer value and conveniently > setting EFLAGS so that the code above just becomes a few loads, AND, branch, > and call. (The size of Flag is unchanged; it grows into the padding only.) > > This is expected to reduce the binary size by ~48KB on Android, and increase it > by 79KB on x64 Linux -- something that's paid for by the split-out refactorings > to WeakPtr and WeakPtrFactory. > > Exposing the SequenceChecker calls in inline functions caused the > MessageLoopTestType*.Nesting test to overflow on Win64 dcheck release builds, > which is why this patch also lowers the recursion depth there. > > BUG= 728324 , 738183 , 738193 > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester > > Review-Url: https://codereview.chromium.org/2963623002 > Cr-Commit-Position: refs/heads/master@{#487048} BUG=748495, 748075 , 744747 Change-Id: Id85da06115ab9453267ade4d6f8efcfd1b26220c Reviewed-on: https://chromium-review.googlesource.com/604088 Commit-Queue: Hans Wennborg <hans@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#492520} [modify] https://crrev.com/4cfa301cda2ab84a496f3c13a5e3fa2f8de94fca/base/memory/weak_ptr.cc [modify] https://crrev.com/4cfa301cda2ab84a496f3c13a5e3fa2f8de94fca/base/memory/weak_ptr.h
,
Aug 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f393cc0504d4da327d541a054c404aa422470c1c commit f393cc0504d4da327d541a054c404aa422470c1c Author: Hans Wennborg <hans@chromium.org> Date: Thu Aug 10 15:45:38 2017 Revert "Make base::WeakPtr::Get() fast" This reverts commit 71f161e3bd95827876f67c656168f936d88a6e3a and 3856eab55bb241fb054a878d9922e65e7b5b0931. Reason for revert: I'm backing out the WeakPtr changes. While the new code was objectively better, it didn't show on any perf bots, regressed size on Linux quite a bit because the new code was smaller and thus inlined more often, and seems to have introduced a new crasher on Android. To summarize: it seems it's not worth it. The WeakPtr revert will be done in two patches. The first part is in https://chromium-review.googlesource.com/c/604087 This second part should be merged to M61. Original change's description: > Make base::WeakPtr::Get() fast > > It was previous calling the out-of-line method WeakReference::is_valid(). > That means that code like > > if (my_weak_ptr) > my_weak_ptr->foo(); > > would do two calls to is_valid(), plus tests and branching. > > The is_valid() method showed up as one of the most frequently called non-inline > functions during browser start-up on Windows (about 1M calls). > > is_valid() was also inefficient in itself, because it had to do a null-check > flag_, as well as checking if the flag was marked valid. > > This patch removes the null-pointer check by using a sentinel Flag object instead of > a nullptr. And instead of storing a bool in the Flag, it stores a pointer-sized > bitmask 0 or ~0, so it can be AND'ed with the pointer value and conveniently > setting EFLAGS so that the code above just becomes a few loads, AND, branch, > and call. (The size of Flag is unchanged; it grows into the padding only.) > > This is expected to reduce the binary size by ~48KB on Android, and increase it > by 79KB on x64 Linux -- something that's paid for by the split-out refactorings > to WeakPtr and WeakPtrFactory. > > Exposing the SequenceChecker calls in inline functions caused the > MessageLoopTestType*.Nesting test to overflow on Win64 dcheck release builds, > which is why this patch also lowers the recursion depth there. > > BUG= 728324 , 738183 , 738193 > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester > > Review-Url: https://codereview.chromium.org/2963623002 > Cr-Commit-Position: refs/heads/master@{#487048} BUG=748495, 748075 , 744747 TBR=hans@chromium.org (cherry picked from commit 4cfa301cda2ab84a496f3c13a5e3fa2f8de94fca) Change-Id: Id85da06115ab9453267ade4d6f8efcfd1b26220c Reviewed-on: https://chromium-review.googlesource.com/604088 Commit-Queue: Hans Wennborg <hans@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#492520} Reviewed-on: https://chromium-review.googlesource.com/610446 Reviewed-by: Hans Wennborg <hans@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#432} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/f393cc0504d4da327d541a054c404aa422470c1c/base/memory/weak_ptr.cc [modify] https://crrev.com/f393cc0504d4da327d541a054c404aa422470c1c/base/memory/weak_ptr.h
,
Aug 10 2017
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Jul 17 2017