Bug 151085 – Print result of memory sampler more readable

• Component: WebKit2
• Status: RESOLVED FIXED

[Gyuyoung Kim]

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

[Gyuyoung Kim]

Created attachment 265154 [details]
Patch

[Gyuyoung Kim]

Created attachment 265156 [details]
Patch

[Darin Adler]

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.

[Gyuyoung Kim]

(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);
}

[Gyuyoung Kim]

Created attachment 265265 [details]
Patch

[Gyuyoung Kim]

Created attachment 265267 [details]
Patch

[Darin Adler]

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(' ');

[Gyuyoung Kim]

Created attachment 265340 [details]
Patch

[Gyuyoung Kim]

Created attachment 265356 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 265356 [details]
Patch

Clearing flags on attachment: 265356

Committed r192349: <http://trac.webkit.org/changeset/192349>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.