Bug 155561 – Replace all of the various non-working and non-compiling sampling profiler hacks with a single super hack

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 274223 [details]
work in progress

[Filip Pizlo]

Created attachment 274247 [details]
maybe the patch

[WebKit Commit Bot]

Attachment 274247 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/SuperSampler.h:33:  g_superSamplerCount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/bytecode/SuperSampler.cpp:36:  g_superSamplerCount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/bytecode/SuperSampler.cpp:48:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 3 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 274307 [details]
better patch

[WebKit Commit Bot]

Attachment 274307 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/SuperSampler.h:33:  g_superSamplerCount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/bytecode/SuperSampler.cpp:36:  g_superSamplerCount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/bytecode/SuperSampler.cpp:48:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 3 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 274314 [details]
the patch?

[WebKit Commit Bot]

Attachment 274314 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/SuperSampler.h:33:  g_superSamplerCount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/bytecode/SuperSampler.cpp:36:  g_superSamplerCount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/bytecode/SuperSampler.cpp:48:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 3 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Saam Barati]

Comment on attachment 274314 [details]
the patch?

View in context: https://bugs.webkit.org/attachment.cgi?id=274314&action=review

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6482
> +        bool sample = false;
> +
> +        if (sample)
> +            m_out.incrementSuperSamplerCount();
> +        

I'm not a big fan of checking this in. I think it's better to let individual users of the profiler do it.
I think this just clutters the code.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/198364

[Csaba Osztrogonác]

(In reply to comment #9)
> Landed in http://trac.webkit.org/changeset/198364

It broke the cloop build: (you could have noticed it on build.webkit.org ...)

/Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/bytecode/SuperSampler.cpp:61:21: error: use of undeclared identifier 'dataLog'
                    dataLog("WARNING: Super sampler undercount detected!\n");
                    ^
/Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/bytecode/SuperSampler.cpp:74:5: error: use of undeclared identifier 'dataLog'
    dataLog("Percent time behind super sampler flag: ", percentage, "\n");
    ^
2 errors generated.

[Csaba Osztrogonác]

(In reply to comment #9)
> Landed in http://trac.webkit.org/changeset/198364

just to document, GTK buildfix landed in https://trac.webkit.org/changeset/198384

[Csaba Osztrogonác]

(In reply to comment #10)
> (In reply to comment #9)
> > Landed in http://trac.webkit.org/changeset/198364
> 
> It broke the cloop build: (you could have noticed it on build.webkit.org ...)
> 
> /Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/
> bytecode/SuperSampler.cpp:61:21: error: use of undeclared identifier
> 'dataLog'
>                     dataLog("WARNING: Super sampler undercount detected!\n");
>                     ^
> /Volumes/Data/slave/elcapitan-cloop-debug/build/Source/JavaScriptCore/
> bytecode/SuperSampler.cpp:74:5: error: use of undeclared identifier 'dataLog'
>     dataLog("Percent time behind super sampler flag: ", percentage, "\n");
>     ^
> 2 errors generated.

Fix landed in https://trac.webkit.org/changeset/198396.
Please check build.webkit.org after landing patches next time.