Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1544983004:
df generation: single allocation with calloc (Closed)
Descriptiondf generation: single allocation with calloc
The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData.
So we've always been spilling to the heap.
If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc.
So in practice this replaces 1-2 malloc, 1-2 free, and 2 bzeros with 1 calloc and 1 free.
BUG=skia:4729
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1544983004
Committed: https://skia.googlesource.com/skia/+/12204d90337656542a42fa0fcccb7bec13af0cce
CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus6-GPU-Adreno420-Arm7-Release-Trybot
Committed: https://skia.googlesource.com/skia/+/2c2d8ab08c39fad2706895e69dcc9ed1aeb9bb2a
Patch Set 1 #Patch Set 2 : simpler #Patch Set 3 : swap #Messages
Total messages: 29 (17 generated)
Description was changed from ========== df generation: single allocation, using calloc if we spill to the heap If we're going to spill to the heap, we might as well allocate these two arrays together, and it's never slower to calloc than malloc+bzero. BUG=skia:4729 ========== to ========== df generation: single allocation, using calloc if we spill to the heap If we're going to spill to the heap, we might as well allocate these two arrays together, and it's never slower to calloc than malloc+bzero. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== df generation: single allocation, using calloc if we spill to the heap If we're going to spill to the heap, we might as well allocate these two arrays together, and it's never slower to calloc than malloc+bzero. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== df generation: single allocation, using calloc if we spill to the heap If we're going to spill to the heap, we might as well allocate/free these two arrays together, and it's never slower to calloc than malloc+bzero. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by mtklein@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/1544983004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1544983004/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== df generation: single allocation, using calloc if we spill to the heap If we're going to spill to the heap, we might as well allocate/free these two arrays together, and it's never slower to calloc than malloc+bzero. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== df generation: single allocation with calloc The dfStorage DFData allocation could never have fit in its stack space before: (4px pad + an extra 1px)*2 sides always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== df generation: single allocation with calloc The dfStorage DFData allocation could never have fit in its stack space before: (4px pad + an extra 1px)*2 sides always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== df generation: single allocation with calloc The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== df generation: single allocation with calloc The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== df generation: single allocation with calloc The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. So in practice this replaces 1 malloc, 1 free, and 2 bzeros with 1 calloc and 1 free. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by mtklein@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/1544983004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1544983004/20001
Description was changed from ========== df generation: single allocation with calloc The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. So in practice this replaces 1 malloc, 1 free, and 2 bzeros with 1 calloc and 1 free. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== df generation: single allocation with calloc The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. So in practice this replaces 1-2 malloc, 1-2 free, and 2 bzeros with 1 calloc and 1 free. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
mtklein@chromium.org changed reviewers: + jvanverth@google.com
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by mtklein@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1544983004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1544983004/20001
Message was sent while issue was closed.
Description was changed from ========== df generation: single allocation with calloc The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. So in practice this replaces 1-2 malloc, 1-2 free, and 2 bzeros with 1 calloc and 1 free. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== df generation: single allocation with calloc The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. So in practice this replaces 1-2 malloc, 1-2 free, and 2 bzeros with 1 calloc and 1 free. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/12204d90337656542a42fa0fcccb7bec13af0cce ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/12204d90337656542a42fa0fcccb7bec13af0cce
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1556263002/ by mtklein@google.com. The reason for reverting is: Crashing multiple bots, e.g. https://uberchromegw.corp.google.com/i/client.skia.android/builders/Perf-Andr....
Message was sent while issue was closed.
Note to self: failures were SIGBUS, trying to load something from DFData (e.g. a float) at an unaligned address (e.g. 0x41d1e677). Simplest solution is to put the edgePtr after the dataPtr, rather than in front.
Description was changed from ========== df generation: single allocation with calloc The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. So in practice this replaces 1-2 malloc, 1-2 free, and 2 bzeros with 1 calloc and 1 free. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/12204d90337656542a42fa0fcccb7bec13af0cce ========== to ========== df generation: single allocation with calloc The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. So in practice this replaces 1-2 malloc, 1-2 free, and 2 bzeros with 1 calloc and 1 free. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/12204d90337656542a42fa0fcccb7bec13af0cce CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus6-GPU-Adreno420-Arm7-Release-Trybot ==========
The CQ bit was checked by mtklein@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jvanverth@google.com Link to the patchset: https://codereview.chromium.org/1544983004/#ps40001 (title: "swap")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1544983004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1544983004/40001
Message was sent while issue was closed.
Description was changed from ========== df generation: single allocation with calloc The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. So in practice this replaces 1-2 malloc, 1-2 free, and 2 bzeros with 1 calloc and 1 free. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/12204d90337656542a42fa0fcccb7bec13af0cce CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus6-GPU-Adreno420-Arm7-Release-Trybot ========== to ========== df generation: single allocation with calloc The dfStorage DFData allocation can never fit in its stack space: 5px padding on each side always implies at least a 10x10 DFData allocation, but the stack space only fits 64 DFData. So we've always been spilling to the heap. If we're going to spill to the heap, we might as well allocate/free all our temporary memory in one block, and since we want it zeroed, might as well calloc. So in practice this replaces 1-2 malloc, 1-2 free, and 2 bzeros with 1 calloc and 1 free. BUG=skia:4729 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/12204d90337656542a42fa0fcccb7bec13af0cce CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus6-GPU-Adreno420-Arm7-Release-Trybot Committed: https://skia.googlesource.com/skia/+/2c2d8ab08c39fad2706895e69dcc9ed1aeb9bb2a ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/2c2d8ab08c39fad2706895e69dcc9ed1aeb9bb2a |
