New issue
Advanced search Search tips

Issue 594758 link

Starred by 1 user

Issue metadata

Status: Archived
Owner: ----
Closed: Jan 10
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Investigate enabling sized deallocation

Project Member Reported by thakis@chromium.org, Mar 14 2016

Issue description

Apparently -fsized-deallocation can help with allocation performance. gcc enables this by default with -std=c++14, but clang doesn't (and we only pass -std=c++11 anyhow).

This probably isn't possible on all platforms since not all standard libraries will have the sized overload, but it's possible on platforms where we control which standard lib is being used (Android, and currently OS X, but we might undo that again). I don't know if MSVS2015's standard library contains these already.


Also, if we placement new anywhere so that the dynamic size of a type doesn't match its static type, things won't work. We might do this in v8.


So maybe more trouble than it's worth, but we should check.



Demo:

$ cat test.cc
struct A {};
int main() {
  delete new A;
}

$ clang  -c test.cc -S -emit-llvm -o - | grep Pv
  call void @_ZdlPv(i8* %1) #4
declare void @_ZdlPv(i8*) #2
$ clang -fsized-deallocation -c test.cc -S -emit-llvm -o - | grep Pv
  call void @_ZdlPvm(i8* %1, i64 1) #4
declare void @_ZdlPvm(i8*, i64) #2


Note how the size of the object is passed to delete in the second example.
 
I wonder whether this could be used to speedup PartitionAlloc, to speed up the bucket (which depends on the alloc size) knowing its size.
Also, we might use it to do something smarter for the browser where we peak 400K malloc / second during pageload and scrolling, but I need to gather more data there, as I suspect we might be doing something terribly silly and we shouldn't malloc at all.

I need to read more. Honestly, I have still lot of conceptual holes in my brain (does this imply an overloaded operator delete?)

> Also, if we placement new anywhere so that the dynamic size of a type doesn't match its static type, things won't work. We might do this in v8
I've seen a bunch of placement in our codebase.
V8 does definitely that with its zone allocator (a fast bump-pointer allocator used for scoped heap bursts which never deletes individual allocs, and deletes the full zone once done). Example:
https://chromium.googlesource.com/v8/v8/+/65b66634de40355498b37dfbdcefd55773ac48a1/src/compiler/node.cc#89

Skia also placement-news:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/skia/src/core/SkData.cpp&q=new%5C%20%5C(.*%5C)%5C%20.*%5C(%20%20f:%5C.c%20-f:v8%20-f:llvm&sq=package:chromium&l=74&ct=rc&cd=3&dr=C

And apparently also something in IPC:
https://code.google.com/p/chromium/codesearch#chromium/src/ipc/ipc_sync_message.cc&l=23


Erratas (I am writing too many emails today):
*to speed up the bucket *lookup*
* 400K malloc / second -> most of these are actually c++ new-s

Comment 3 by thakis@chromium.org, Mar 17 2016

we explicitly pass '/Zc:sizedDealloc-' to cl.exe due to bug 526851 at the moment.

Comment 4 by thakis@chromium.org, Dec 12 2016

Bruce reports in https://codereview.chromium.org/2552623002/ that /Zc:sizedDealloc- saves us a quarter MB of code on Windows (!) So we might want to not turn this on after all.
On the official Windows builds (everything except PGO) the cost of enabling sizedDealloc is only 190 KB. But still, that's annoying.

So, we can enable this feature if we need it, but there is some cost.
Status: Archived (was: Untriaged)
Archiving issues older than 2 years with no owner or component.

Sign in to add a comment