| Summary: | TypeProfiler and TypeProfilerLog don't play nicely with the concurrent JIT | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||||
| Component: | JavaScriptCore | Assignee: | Saam Barati <saam> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | bburg, benjamin, commit-queue, fpizlo, ggaren, gskachkov, joepeck, keith_miller, mark.lam, msaboff, nvasilyev, sukolsak, timothy, webkit-bug-importer, ysuzuki | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Saam Barati
2016-06-22 13:52:16 PDT
Created attachment 281882 [details]
patch
Attachment 281882 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/TypeSet.cpp:52: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 1 in 11 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 281882 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=281882&action=review > Source/JavaScriptCore/runtime/TypeSet.h:106 > +public: > + ConcurrentJITLock m_lock; Why put the lock in the middle instead of in the public declarations above? Comment on attachment 281882 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=281882&action=review >> Source/JavaScriptCore/runtime/TypeSet.h:106 >> + ConcurrentJITLock m_lock; > > Why put the lock in the middle instead of in the public declarations above? They fit into 32-bits this way. This won't matter for 64 bit paltforms, but will save 32-bits in TypeSet size for 32-bit platforms. I guess I could reorder the fields like so to prevent the private/public break: ``` public: ... methods and such ... Lock m_lock; private: bool m_isOverflown; RuntimeTypeMask ... .... ``` Comment on attachment 281882 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=281882&action=review >>> Source/JavaScriptCore/runtime/TypeSet.h:106 >>> + ConcurrentJITLock m_lock; >> >> Why put the lock in the middle instead of in the public declarations above? > > They fit into 32-bits this way. This won't matter for 64 bit paltforms, but will save 32-bits in TypeSet size for 32-bit platforms. > I guess I could reorder the fields like so to prevent the private/public break: > ``` > public: > ... methods and such > ... > Lock m_lock; > private: > bool m_isOverflown; > RuntimeTypeMask ... > .... > ``` I'll go with this reordering. Created attachment 281886 [details]
patch for landing
Attachment 281886 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/TypeSet.cpp:52: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 1 in 11 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 281890 [details]
patch for landing that builds
Attachment 281890 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/TypeSet.cpp:52: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 1 in 11 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 281890 [details] patch for landing that builds Clearing flags on attachment: 281890 Committed r202364: <http://trac.webkit.org/changeset/202364> All reviewed patches have been landed. Closing bug. |