Bug 155561

Summary:

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

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, benjamin, commit-queue, ggaren, keith_miller, mark.lam, mhahnenb, msaboff, oliver, ossy, saam, sam

Priority:

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
work in progress	none
maybe the patch	none
better patch	none
the patch?	saam: review+

Filip Pizlo

Reported 2016-03-16 14:59:04 PDT

Patch forthcoming.

Attachments
work in progress (110.70 KB, patch) 2016-03-16 14:59 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
maybe the patch (126.67 KB, patch) 2016-03-16 17:56 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
better patch (126.85 KB, patch) 2016-03-17 11:56 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch? (127.83 KB, patch) 2016-03-17 12:28 PDT, Filip Pizlo	saam: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2016-03-16 14:59:34 PDT

Created attachment 274223 [details] work in progress

Filip Pizlo

Comment 2 2016-03-16 17:56:21 PDT

Created attachment 274247 [details] maybe the patch

WebKit Commit Bot

Comment 3 2016-03-16 17:58:48 PDT

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

Comment 4 2016-03-17 11:56:18 PDT

Created attachment 274307 [details] better patch

WebKit Commit Bot

Comment 5 2016-03-17 12:00:40 PDT

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

Comment 6 2016-03-17 12:28:36 PDT

Created attachment 274314 [details] the patch?

WebKit Commit Bot

Comment 7 2016-03-17 12:30:16 PDT

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 8 2016-03-17 13:43:23 PDT

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

Comment 9 2016-03-17 17:54:41 PDT

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

Csaba Osztrogonác

Comment 10 2016-03-18 00:21:56 PDT

(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

Comment 11 2016-03-18 00:23:09 PDT

(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

Comment 12 2016-03-18 04:41:34 PDT

(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.

Note You need to log in before you can comment on or make changes to this bug.