Bug 159037 – TypeProfiler and TypeProfilerLog don't play nicely with the concurrent JIT

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

[Saam Barati]

We process the log on a background compiler thread, and this leads to a bad time

[Saam Barati]

<rdar://problem/26935349>

[Saam Barati]

Created attachment 281882 [details]
patch

[WebKit Commit Bot]

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.

[Benjamin Poulain]

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?

[Saam Barati]

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

[Saam Barati]

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.

[Saam Barati]

Created attachment 281886 [details]
patch for landing

[WebKit Commit Bot]

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.

[Saam Barati]

Created attachment 281890 [details]
patch for landing that builds

[WebKit Commit Bot]

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.

[WebKit Commit Bot]

Comment on attachment 281890 [details]
patch for landing that builds

Clearing flags on attachment: 281890

Committed r202364: <http://trac.webkit.org/changeset/202364>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.