1294683 - The eval cache isn't being hit

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

I tried to come up with a testcase for bug 1294566 but couldn't find a way to get a hit in the eval cache because IsEvalCacheCandidate always seems to return false.

 static bool
 IsEvalCacheCandidate(JSScript* script)
 {
    // Make sure there are no inner objects which might use the wrong parent
    // and/or call scope by reusing the previous eval's script. Skip the
    // script's first object, which entrains the eval's scope.
    return script->savedCallerFun() &&
           !script->hasSingletons() &&
           script->objects()->length == 1;
 }

In situations where the first two conditions pass I always see two objects in the script->objects() array: a JSFunction and a StaticBlockScope.

jandem suggested that the checks on the length of the inner object array were out of date and might not make sense any more.

I tested the sunspider date-format-tofte test and this doesn't hit the eval cache either.

Comment 1

[Nicolas B. Pierron [:nbp]]

From what I recall the eval cache was made to improve the performance of one sunsipder benchmark which generate function calls like:

  eval(name + "()");

Maybe double check with sunspider, if you have any hit, or if there is an issue.

Comment 2

[Shu-yu Guo [:shu]]

(In reply to Jon Coppeard (:jonco) from comment #0)
> I tried to come up with a testcase for bug 1294566 but couldn't find a way
> to get a hit in the eval cache because IsEvalCacheCandidate always seems to
> return false.
> 
>  static bool
>  IsEvalCacheCandidate(JSScript* script)
>  {
>     // Make sure there are no inner objects which might use the wrong parent
>     // and/or call scope by reusing the previous eval's script. Skip the
>     // script's first object, which entrains the eval's scope.
>     return script->savedCallerFun() &&
>            !script->hasSingletons() &&
>            script->objects()->length == 1;
>  }
> 
> In situations where the first two conditions pass I always see two objects
> in the script->objects() array: a JSFunction and a StaticBlockScope.
> 
> jandem suggested that the checks on the length of the inner object array
> were out of date and might not make sense any more.
> 
> I tested the sunspider date-format-tofte test and this doesn't hit the eval
> cache either.

Jan's IRC comments were right, the eval static scope stuff I did a while back accidentally disabled the eval cache.

Bug 1263355 moved scopes into their own GC thing and their own array, and thus re-enabled the eval cache, and surfaced the bug I saw in bug 1294566. So don't worry about the eval cache not working for now, bug 1263355 (inadvertently) fixed it.

Comment 3

[Jon Coppeard (:jonco)]

This will be fixed by bug 1263355.