Summary: | Print result of memory sampler more readable | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||||||
Component: | WebKit2 | Assignee: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | commit-queue, darin | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Gyuyoung Kim
2015-11-10 00:59:18 PST
Created attachment 265154 [details]
Patch
Created attachment 265156 [details]
Patch
Comment on attachment 265156 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265156&action=review > Source/WebKit2/Shared/WebMemorySampler.cpp:171 > + statString.append(String::format("%-35s", memoryStats.keys[i].utf8().data())); Sure would be nice to do this without String::format. We really want to get rid of it entirely because of the performance and security problems with it. I think we could write something that does the padding a different way: appendSpaces(statString, 35 - memoryStats.keys[i].length()); statString.append(memoryStats.keys[i]); Then we just have to write that helper function. (In reply to comment #3) > Comment on attachment 265156 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=265156&action=review > > > Source/WebKit2/Shared/WebMemorySampler.cpp:171 > > + statString.append(String::format("%-35s", memoryStats.keys[i].utf8().data())); > > Sure would be nice to do this without String::format. We really want to get > rid of it entirely because of the performance and security problems with it. > I think we could write something that does the padding a different way: > > appendSpaces(statString, 35 - memoryStats.keys[i].length()); > statString.append(memoryStats.keys[i]); > > Then we just have to write that helper function. Below appendSpaces() works well. However I think we can write this function with better method. Any suggestion ? static const char space = ' '; static void appendSpaces(StringBuilder& key, const int spaceCount) { for (int i = 0; i < spaceCount; ++i) key.append(space); } Created attachment 265265 [details]
Patch
Created attachment 265267 [details]
Patch
Comment on attachment 265267 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265267&action=review > Source/WebKit2/Shared/WebMemorySampler.cpp:42 > +static void appendSpaces(StringBuilder& string, const int count) No reason for the "const" in "const int". > Source/WebKit2/Shared/WebMemorySampler.cpp:44 > + char space[] = " "; No reason to use a string literal instead of a single character. > Source/WebKit2/Shared/WebMemorySampler.cpp:46 > + string.appendLiteral(space); Could be: string.append(' '); Created attachment 265340 [details]
Patch
Created attachment 265356 [details]
Patch
Comment on attachment 265356 [details] Patch Clearing flags on attachment: 265356 Committed r192349: <http://trac.webkit.org/changeset/192349> All reviewed patches have been landed. Closing bug. |