Current memory sampler prints result of memory sample horizontally. It makes us hard to read the result at a glance. To improve readability of the memory sampler result, this patch prints it vertically. = Print result now = Process: WebProcess Pid: 18681 Timestamp Total Program Size RSS Shared Text Library Data/Stack Dirty JavaScript Heap In Use JavaScript Heap Commited Memory JavaScript Stack Bytes JavaScript JIT Bytes Total Memory In Use Total Committed Memory System Total Bytes Available Bytes Shared Bytes Buffer Bytes Total Swap Bytes Available Swap Bytes 1447145886 774583 36266 20133 1 0 666682 0 1163112 3840880 5264 585728 1754104 4431872 16731484160 5025890304 99672064 571256832 7999995904 7999995904 1447145887 778861 40865 20322 1 0 670960 0 8074138 10167970 5264 1314816 9394218 11488050 16731484160 5011161088 99790848 571265024 7999995904 7999995904 = Print result after landing this patch = Process: WebProcess Pid 16972 Timestamp 1447145164 Total Program Size 778988 RSS 40010 Shared 20312 Text 1 Library 0 Data/Stack 671087 Dirty 0 JavaScript Heap In Use 4152244 JavaScript Heap Commited Memory 7884228 JavaScript Stack Bytes 4288 JavaScript JIT Bytes 1269760 Total Memory In Use 5426292 Total Committed Memory 9158276 System Total Bytes 16731484160 Available Bytes 5091196928 Shared Bytes 98856960 Buffer Bytes 569925632 Total Swap Bytes 7999995904 Available Swap Bytes 7999995904
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.