Use a filter instead of a visitor to deoptimize selected functions in a context.

Use a filter instead of a visitor to deoptimize selected functions in a context.

This makes the DeoptimizeAll function O(n) instead of O(n^2) where n in the number of optimized functions.

Before this change, DeoptimizeAll iterated over the optimized function list and called DeoptimizingVisitor for each function. The visitor iterated over the optimized function list again to remove the functions that share the same optimized code.

This change partitions the optimized function list into one or more lists of related functions in one pass over the optimized function list.

R=mstarzinger@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=13226

[ulan, 2012-12-12 13:50:57]

Please take a look. This is needed for the upcoming "weak maps in optimized
code" CL.

https://chromiumcodereview.appspot.com/11547015/diff/4010/src/arm/deoptimizer...
File src/arm/deoptimizer-arm.cc (right):

https://chromiumcodereview.appspot.com/11547015/diff/4010/src/arm/deoptimizer...
src/arm/deoptimizer-arm.cc:49: Isolate* isolate = function->GetIsolate();
This changes are to sync code for different architectures.

[Sven Panne, 2012-12-12 14:31:48]

Quick DBC...

https://chromiumcodereview.appspot.com/11547015/diff/4010/src/deoptimizer.h
File src/deoptimizer.h (right):

https://chromiumcodereview.appspot.com/11547015/diff/4010/src/deoptimizer.h#n...
src/deoptimizer.h:90: class OptimizedFunctionFilter BASE_EMBEDDED {
This is what is commonly called a predicate (even in STL ;-), and it might be
nicer to move this in a templatized version to utils.h. Perhaps we should even
add a unary_function template there from which can subclass.
OptimizedFunctionFilter can be a typedef if it is needed at all then.

Anyway, overloading function invocation (i.e. operator()) seems to be more
natural than making up a name like TakeFunction.

[Michael Starzinger, 2012-12-12 14:33:54]

LGTM. I like this change!

[Michael Starzinger, 2012-12-12 14:39:42]

https://codereview.chromium.org/11547015/diff/4010/src/deoptimizer.h
File src/deoptimizer.h (right):

https://codereview.chromium.org/11547015/diff/4010/src/deoptimizer.h#newcode90
src/deoptimizer.h:90: class OptimizedFunctionFilter BASE_EMBEDDED {
On 2012/12/12 14:31:48, Sven Panne wrote:
> This is what is commonly called a predicate (even in STL ;-), and it might be
> nicer to move this in a templatized version to utils.h. Perhaps we should even
> add a unary_function template there from which can subclass.
> OptimizedFunctionFilter can be a typedef if it is needed at all then.
> 
> Anyway, overloading function invocation (i.e. operator()) seems to be more
> natural than making up a name like TakeFunction.

I am fine with both solutions (a templetized filter/predicate or a type-specific
filter). Given that we already have several of these home-grown filters (e.g.
HeapObjectsFilter) I have no problem with adding another one. Having one
templetized class is also fine with me. My opinion is not strong enough to sway
either way.

[ulan_google, 2012-12-12 15:00:46]

https://codereview.chromium.org/11547015/diff/4010/src/deoptimizer.h
File src/deoptimizer.h (right):

https://codereview.chromium.org/11547015/diff/4010/src/deoptimizer.h#newcode90
src/deoptimizer.h:90: class OptimizedFunctionFilter BASE_EMBEDDED {
On 2012/12/12 14:39:42, Michael Starzinger wrote:
> On 2012/12/12 14:31:48, Sven Panne wrote:
> > This is what is commonly called a predicate (even in STL ;-), and it might
be
> > nicer to move this in a templatized version to utils.h. Perhaps we should
even
> > add a unary_function template there from which can subclass.
> > OptimizedFunctionFilter can be a typedef if it is needed at all then.
> > 
> > Anyway, overloading function invocation (i.e. operator()) seems to be more
> > natural than making up a name like TakeFunction.
> 
> I am fine with both solutions (a templetized filter/predicate or a
type-specific
> filter). Given that we already have several of these home-grown filters (e.g.
> HeapObjectsFilter) I have no problem with adding another one. Having one
> templetized class is also fine with me. My opinion is not strong enough to
sway
> either way.
After offline discussion, I am leaving this as it is for now.