857725 - BaselineCompiler: Minor cleanup

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Attachment: Patch

I've had this patch in my queue for a while. Removes some TODO comments (looks like we don't need megamorphic stubs), and lowers the max number of stubs we attach for some IC's.

Comment 1

[Kannan Vijayan [:djvj]]

Comment on attachment 732975 [details] [diff] [review]
Patch

Review of attachment 732975 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/BaselineIC.cpp
@@ -1012,5 @@
>      JS_ASSERT_IF(wasDetachedMonitorChain, numOptimizedMonitorStubs_ == 0);
>  
> -    if (numOptimizedMonitorStubs_ >= MAX_OPTIMIZED_STUBS) {
> -        // TODO: if the TypeSet becomes unknown or has the AnyObject type,
> -        // replace stubs with a single stub to handle these.

I would like to leave this TODO here.  But first we want to reduce the maximum type object count held by a TypeSet down to something reasonable (like, 32 instead of the current 256).

@@ -1244,5 @@
>                                       RawId id, HandleValue val)
>  {
> -    if (numOptimizedStubs_ >= MAX_OPTIMIZED_STUBS) {
> -        // TODO: if the TypeSet becomes unknown or has the AnyObject type,
> -        // replace stubs with a single stub to handle these.

Same as above.

::: js/src/ion/BaselineIC.h
@@ +4479,2 @@
>      static const uint32_t MAX_SCRIPTED_STUBS = 7;
>      static const uint32_t MAX_NATIVE_STUBS = 7;

I think a better change for this would be to do:

MAX_SCRIPTED_STUBS=5
MAX_NATIVE_STUBS=5
MAX_OPTIMIZED_STUBS = MAX_SCRIPTED_STUBS + MAX_NATIVE_STUBS + 1;

This will always leave opportunity for native and scripted call stubs to collapse into a generic.

Comment 2

[Nicolas B. Pierron [:nbp]]

Any plan on landing this patch?  Is this bug still valid?

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Nicolas B. Pierron [:nbp] from comment #2)
> Any plan on landing this patch?  Is this bug still valid?

Yup I will try to land it this week.