WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 174919
Primitive auxiliaries and JSValue auxiliaries should have separate gigacages
https://bugs.webkit.org/show_bug.cgi?id=174919
Summary
Primitive auxiliaries and JSValue auxiliaries should have separate gigacages
Filip Pizlo
Reported
2017-07-27 19:38:07 PDT
...
Attachments
it begins
(33.59 KB, patch)
2017-08-06 10:40 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
it is written
(62.78 KB, patch)
2017-08-06 11:18 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
starting to compile
(71.10 KB, patch)
2017-08-06 11:29 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
more
(72.87 KB, patch)
2017-08-06 11:43 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
a bit more
(112.99 KB, patch)
2017-08-06 12:09 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
the patch
(136.56 KB, patch)
2017-08-06 15:29 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
the patch
(136.41 KB, patch)
2017-08-07 10:24 PDT
,
Filip Pizlo
keith_miller
: review+
Details
Formatted Diff
Diff
patch for landing
(136.52 KB, patch)
2017-08-07 14:24 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
patch for landing
(136.62 KB, patch)
2017-08-07 14:27 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
Show Obsolete
(8)
View All
Add attachment
proposed patch, testcase, etc.
Filip Pizlo
Comment 1
2017-08-06 10:40:12 PDT
Created
attachment 317367
[details]
it begins
Filip Pizlo
Comment 2
2017-08-06 11:18:17 PDT
Created
attachment 317369
[details]
it is written Maybe?
Filip Pizlo
Comment 3
2017-08-06 11:29:16 PDT
Created
attachment 317372
[details]
starting to compile
Filip Pizlo
Comment 4
2017-08-06 11:43:51 PDT
Created
attachment 317373
[details]
more
Filip Pizlo
Comment 5
2017-08-06 12:09:50 PDT
Created
attachment 317374
[details]
a bit more
Filip Pizlo
Comment 6
2017-08-06 15:29:14 PDT
Created
attachment 317385
[details]
the patch
Build Bot
Comment 7
2017-08-06 15:32:12 PDT
Attachment 317385
[details]
did not pass style-queue: ERROR: Source/WTF/wtf/Gigacage.h:76: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:79: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:80: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:88: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:93: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:94: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:37: g_primitiveGigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:38: g_jsValueGigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/BInline.h:26: Use #pragma once instead of #ifndef for header guard. [build/header_guard] [5] ERROR: Source/bmalloc/bmalloc/Cache.cpp:26: Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 13 in 55 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 8
2017-08-07 10:24:58 PDT
Created
attachment 317432
[details]
the patch
Build Bot
Comment 9
2017-08-07 10:27:05 PDT
Attachment 317432
[details]
did not pass style-queue: ERROR: Source/WTF/wtf/Gigacage.h:76: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:79: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:80: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:88: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:93: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:94: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:37: g_primitiveGigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:38: g_jsValueGigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/BInline.h:26: Use #pragma once instead of #ifndef for header guard. [build/header_guard] [5] ERROR: Source/bmalloc/bmalloc/Cache.cpp:26: Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 13 in 55 files If any of these errors are false positives, please file a bug against check-webkit-style.
Keith Miller
Comment 10
2017-08-07 10:55:36 PDT
Comment on
attachment 317432
[details]
the patch View in context:
https://bugs.webkit.org/attachment.cgi?id=317432&action=review
r=me.
> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11623 > + if (kind == Gigacage::Primitive) { > + if (vm().primitiveGigacageEnabled().isStillValid()) > + m_graph.watchpoints().addLazily(vm().primitiveGigacageEnabled()); > + else > + return ptr;
Just to double check. We don't need this for the JSValue Gigacage because it cannot be disabled?
Filip Pizlo
Comment 11
2017-08-07 14:24:21 PDT
(In reply to Keith Miller from
comment #10
)
> Comment on
attachment 317432
[details]
> the patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=317432&action=review
> > r=me. > > > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11623 > > + if (kind == Gigacage::Primitive) { > > + if (vm().primitiveGigacageEnabled().isStillValid()) > > + m_graph.watchpoints().addLazily(vm().primitiveGigacageEnabled()); > > + else > > + return ptr; > > Just to double check. We don't need this for the JSValue Gigacage because it > cannot be disabled?
Yup!
Filip Pizlo
Comment 12
2017-08-07 14:24:49 PDT
Created
attachment 317465
[details]
patch for landing
Filip Pizlo
Comment 13
2017-08-07 14:27:57 PDT
Created
attachment 317467
[details]
patch for landing
Filip Pizlo
Comment 14
2017-08-07 14:30:48 PDT
I think I fixed the Windows build issues, but now Windows EWS has gone away. I'll land and ask questions later.
Build Bot
Comment 15
2017-08-07 14:31:54 PDT
Attachment 317467
[details]
did not pass style-queue: ERROR: Source/WTF/wtf/Gigacage.h:76: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:79: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:80: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:88: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:93: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:94: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:37: g_primitiveGigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:38: g_jsValueGigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/BInline.h:26: Use #pragma once instead of #ifndef for header guard. [build/header_guard] [5] ERROR: Source/bmalloc/bmalloc/Cache.cpp:26: Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 14 in 55 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 16
2017-08-07 14:32:29 PDT
Landed in
https://trac.webkit.org/changeset/220352/webkit
Radar WebKit Bug Importer
Comment 17
2017-08-07 14:32:55 PDT
<
rdar://problem/33761863
>
Carlos Alberto Lopez Perez
Comment 18
2017-08-07 17:22:44 PDT
(In reply to Filip Pizlo from
comment #16
)
> Landed in
https://trac.webkit.org/changeset/220352/webkit
It seems this has caused around 3600 new failures on the Linux JSC tests. * GTK+ test run at
r220351
(jscore-test step green)
https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/2495
* GTK+ test run at
r220352
(3645 JSC tests failed)
https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/2496
* WPE test run at
r220351
(jscore-test step green)
https://build.webkit.org/builders/WPE%20Linux%2064-bit%20Release%20%28Tests%29/builds/1922
* WPE test run at
r220352
(3623 JSC tests failed)
https://build.webkit.org/builders/WPE%20Linux%2064-bit%20Release%20%28Tests%29/builds/1923
Carlos Alberto Lopez Perez
Comment 19
2017-08-07 17:30:53 PDT
(In reply to Carlos Alberto Lopez Perez from
comment #18
)
> (In reply to Filip Pizlo from
comment #16
) > > Landed in
https://trac.webkit.org/changeset/220352/webkit
> > It seems this has caused around 3600 new failures on the Linux JSC tests. > > * GTK+ test run at
r220351
(jscore-test step green) >
https://build.webkit.org/builders/GTK%20Linux%2064
- > bit%20Release%20%28Tests%29/builds/2495 > * GTK+ test run at
r220352
(3645 JSC tests failed) >
https://build.webkit.org/builders/GTK%20Linux%2064
- > bit%20Release%20%28Tests%29/builds/2496 > > * WPE test run at
r220351
(jscore-test step green) >
https://build.webkit.org/builders/WPE%20Linux%2064
- > bit%20Release%20%28Tests%29/builds/1922 > * WPE test run at
r220352
(3623 JSC tests failed) >
https://build.webkit.org/builders/WPE%20Linux%2064
- > bit%20Release%20%28Tests%29/builds/1923
And
r220368
fixed it? WPE bot is back to green after it:
https://build.webkit.org/builders/WPE%20Linux%2064-bit%20Release%20%28Tests%29/builds/1928
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug