Investigate saving binary size by changing const char[] foo -> const char* foo |
|||||
Issue descriptionDiscovered this from: https://crrev.com/d9945242d106d9aae4765d236f4b6195a3292ae6 This commit's patch is: --- a/third_party/WebKit/Source/core/css/CSSValue.h +++ b/third_party/WebKit/Source/core/css/CSSValue.h @@ -40,7 +40,7 @@ static void* allocateObject(size_t size, bool isEager) { ThreadState* state = ThreadStateFor<ThreadingTrait<CSSValue>::Affinity>::state(); - const char typeName[] = "blink::CSSValue"; + const char* typeName = "blink::CSSValue"; return ThreadHeap::allocateOnArenaIndex( state, size, isEager ? BlinkGC::EagerSweepArenaIndex : BlinkGC::CSSValueArenaIndex, --- a/third_party/WebKit/Source/core/dom/Node.h +++ b/third_party/WebKit/Source/core/dom/Node.h @@ -164,7 +164,7 @@ static void* allocateObject(size_t size, bool isEager) { ThreadState* state = ThreadStateFor<ThreadingTrait<Node>::Affinity>::state(); - const char typeName[] = "blink::Node"; + const char* typeName = "blink::Node"; return ThreadHeap::allocateOnArenaIndex( state, size, isEager ? BlinkGC::EagerSweepArenaIndex : BlinkGC::NodeArenaIndex, And yet it shrank libchrome.so by 10kb! Worth investigating whether there are more savings to be had here, and maybe adding a rule to the style guide about [] vs *.
,
Mar 17 2017
It is not possible to change the * to []. Would you read the commit comment?
,
Mar 17 2017
,
Mar 17 2017
I think, this depends on android-gcc optimization.
,
Mar 17 2017
I'm sorry. I misunderstood this issue.
The code is:
some inline function/method() {
const char array[] = " .... ";
...
}
So when using android-gcc, currently the array is created on stack and a code which copying "...." to stack is generated everywhere the function / method is used.
# When using clang, such copying seems to not occur.
,
Mar 17 2017
Interesting. Thanks for digging in! Probably still worth doing some amount of investigation in our codebase to see whether "const char[]" is being used unnecessarily, but perhaps this should wait until Android has switched to clang.
,
Mar 27 2017
Yeah, I think, still worth seeing "const char[]" inside methods or functions. Because (talking about my patch) the "const char []"-s should be "static const char []" or "const char*". So I fixed.
,
May 9 2017
,
Jul 20 2017
Android has switched to clang now, so likely this is not an interesting avenue to spend time on. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by agrieve@chromium.org
, Mar 16 2017