Bug 159037 - TypeProfiler and TypeProfilerLog don't play nicely with the concurrent JIT
Summary: TypeProfiler and TypeProfilerLog don't play nicely with the concurrent JIT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-06-22 13:52 PDT by Saam Barati
Modified: 2016-06-22 20:24 PDT (History)
15 users (show)

See Also:


Attachments
patch (55.84 KB, patch)
2016-06-22 16:33 PDT, Saam Barati
benjamin: review+
Details | Formatted Diff | Diff
patch for landing (55.86 KB, patch)
2016-06-22 16:57 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch for landing that builds (56.04 KB, patch)
2016-06-22 17:46 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2016-06-22 13:52:16 PDT
We process the log on a background compiler thread, and this leads to a bad time
Comment 1 Saam Barati 2016-06-22 16:06:06 PDT
<rdar://problem/26935349>
Comment 2 Saam Barati 2016-06-22 16:33:17 PDT
Created attachment 281882 [details]
patch
Comment 3 WebKit Commit Bot 2016-06-22 16:35:09 PDT
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 4 Benjamin Poulain 2016-06-22 16:45:22 PDT
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 5 Saam Barati 2016-06-22 16:51:03 PDT
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 6 Saam Barati 2016-06-22 16:51:49 PDT
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.
Comment 7 Saam Barati 2016-06-22 16:57:52 PDT
Created attachment 281886 [details]
patch for landing
Comment 8 WebKit Commit Bot 2016-06-22 16:59:22 PDT
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.
Comment 9 Saam Barati 2016-06-22 17:46:14 PDT
Created attachment 281890 [details]
patch for landing that builds
Comment 10 WebKit Commit Bot 2016-06-22 17:47:54 PDT
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 11 WebKit Commit Bot 2016-06-22 20:24:31 PDT
Comment on attachment 281890 [details]
patch for landing that builds

Clearing flags on attachment: 281890

Committed r202364: <http://trac.webkit.org/changeset/202364>
Comment 12 WebKit Commit Bot 2016-06-22 20:24:37 PDT
All reviewed patches have been landed.  Closing bug.