[webkit-reviews] review granted: [Bug 134860] Make improvements to Type Profiling : [Attachment 235332] patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Saam Barati <sbarati at apple.com>'s
request for review:
Bug 134860: Make improvements to Type Profiling
https://bugs.webkit.org/show_bug.cgi?id=134860

Attachment 235332: patch
https://bugs.webkit.org/attachment.cgi?id=235332&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=235332&action=review


I'll land this with the style changes.

> Source/JavaScriptCore/runtime/Executable.cpp:515
> +	   if (vm.isProfilingTypesWithHighFidelity())
> +	      
vm.highFidelityTypeProfiler()->functionHasExecutedCache()->insertUnexecutedRang
e(sourceID(), 
> +		  
unlinkedFunctionExecutable->highFidelityTypeProfilingStartOffset(), 
> +		  
unlinkedFunctionExecutable->highFidelityTypeProfilingEndOffset());

This should have { } around the body of the if, since the body is more than one
line.

> Source/JavaScriptCore/runtime/Structure.cpp:1083
> +		   // FIXME: What is a custom property slot?

A custom property slot is something that cannot be created from JavaScript. 
There are two kinds of them:

1) Someone inherits from JSObject and overrides getOwnPropertySlot so that it
intercepts attempts to access certain properties, and then does "custom" things
instead.  This allows you to create properties with magical semantics. 
Array.length is an example of a custom property.  Note that custom properties
will have behavior that is observably different from anything that you could
create in JavaScript.

2) Custom getter property.  This is really just a getter, except that instead
of the getter being a JavaScript function, it's a C function.  Note that custom
getters are not observably any different from normal getters.  They are sort of
a redundant feature since we can also have JavaScript functions that are backed
by native code, so you could create the equivalent of a custom getter by having
a normal getter with a JSFunction that has a NativeExecutable.

I will remove this comment.

> Source/JavaScriptCore/runtime/TypeLocationCache.h:46
> +		      && m_sourceID == other.m_sourceID
> +		      && m_start == other.m_start
> +		      && m_end == other.m_end;

Bad indentation.  Only four spaces, so:

return m_globalVariableID == other.m_globalVariableID
    && m_sourceID == other.m_sourceID
...