Inline Array.map/filter methods

[nadako]

As we discussed in Slack, it would be nice to inline array map and filter methods so we can avoid function calls and closure creation when we use anonymous functions.

With this, if we write a.map(function(i) return i * 2), we get:

C++

HXLINE(   6)        HX_VARI( ::Array< Int >,result) = ::Array_obj< Int >::__new(0);
HXDLIN(   6)        _hx_array_set_size_exact(result,a->length);
HXDLIN(   6)        {
HXLINE(   6)            HX_VARI( Int,_g1) = (int)0;
HXDLIN(   6)            HX_VARI( Int,_g) = a->length;
HXDLIN(   6)            while((_g1 < _g)){
HXLINE(   6)                _g1 = (_g1 + (int)1);
HXDLIN(   6)                HX_VARI( Int,i) = (_g1 - (int)1);
HXDLIN(   6)                {
HXLINE(   6)                    Int inValue = (( (Int)(_hx_array_unsafe_get(a,i)) ) * (int)2);
HXDLIN(   6)                    result->__unsafe_set(i,inValue);
                            }
                        }
                    }

JS:

    var a1 = new Array(a.length);
    var _g1 = 0;
    var _g = a.length;
    while(_g1 < _g) {
        var i = _g1++;
        a1[i] = a[i] * 2;
    }

It's similar in other targets (also note the preallocation optimization for map). And it still works through Dynamic/structure typing, because actual Array objects have these methods as well (they are marked with @:runtime for extern Array's so they really work).

In Java I had to use __get/__set methods that do bounds checking instead of faster __unsafe_get/set because of #5748. Let's change that when it's fixed.

[nadako]

Oops, C# fails at the moment, partly due to #5747

[Simn]

What's the state here?

[nadako]

Oh, I have to check.

[RealyUniqueName]

map is already inlined for php7. filter could be reworked to completely eliminate function calls.

[RealyUniqueName]

Up :)

[nadako]

Rebased. C# tests still fail with an issue that looks like #5748

[RealyUniqueName]

This PR is affected by 11186ef. Anonymous functions are not inlined anymore.

[Simn]

This would need an update.

[Simn]

This is blocked on #5748 (and on @nadako updating it).

[RealyUniqueName]

Updated. Let's do this for 4.0

[RealyUniqueName]

Blocked by #8654

[RealyUniqueName]

Ready to go

[Aurel300]

Strange spacing here.

[jcward]

Awesome!

For the curious like me, the CPP and JS output of the OP example -- prior to this merge -- generates closures, something like:

C++:

            	HX_BEGIN_LOCAL_FUNC_S0(hx::LocalFunc,_hx_Closure_0) HXARGC(1)
            	int _hx_run(int i){
            		HX_STACKFRAME(&_hx_pos_df13cafffbed243f_6_main)
HXLINE(   6)		return (i * 2);
            	}
            	HX_END_LOCAL_FUNC1(return)

            	HX_STACKFRAME(&_hx_pos_df13cafffbed243f_3_main)
HXLINE(   6)		a->map( ::Dynamic(new _hx_Closure_0()));
            	}

JS:

	var a = [1,2,3];
	var b = a.map(function(i) {
		return i * 2;
	});

I suspect this won't affect node / chrome / V8 drastically, but CPP (and HL?) could see a nice gain.

[RealyUniqueName]

Actually it makes huge difference even on node:

import haxe.Timer;

class Main {
	public static function main():Void {
		var a = [for(i in 0...0xFFF) i];
		var b;
		Timer.measure(() -> for(i in 0...0xFFF) b = a.map(i -> i));
		// Timer.measure(() -> for(i in 0...0xFFFF) b = js.Syntax.code('{0}.map({1})', a, i -> i));
	}
}

The first line result (with this patch in effect)

$ haxe -main Main -js test.js && node test.js
Main.hx:7: 0.0690000057220459s

The second line result (without this patch)

$ haxe -main Main -js test.js && node test.js
Main.hx:8: 0.7279999256134033s

Edit:
Sorry, because of a typo the first line runs for 0xFFF iterations while the second line runs for 0xFFFF iterations.
With the same amount of iterations 0xFFFF the first line (with this patch) is even worse than no inlining:

$ haxe -main Main -js test.js && node test.js
Main.hx:7: 1.0940001010894775s

[RealyUniqueName]

Should I disable inlining for js?

[RealyUniqueName]

Though for a single map call this patch is almost nine times faster.

[jcward]

Ok, I stand corrected! Many performance tweaks I've played with have little affect on V8 (presumably they've invested heavily in optimizations). I expected they'd be able to optimize away the inner function call, but I'm surprised not:

https://jsperf.com/inlining-map-function shows 5X faster in my Chrome 73 browser:

Interestingly, when I run basically the same test in node, I'm seeing it 26X faster:

>node inline_map_test.js
   Loop: avg 4.28 ms
Closure: avg 112.47 ms

I don't have this patch yet, but I'm interesting in CPP and HL.

[RealyUniqueName]

Ah, that's because we create new array like [] instead of new Array(length). But new Array(length) comes with slower item access afterwards: #8040 (comment)

[RealyUniqueName]

I wonder if creating an array with new Array(length) and then returning a.copy() would gain benefits of both sides. Maybe V8 could figure out the type of items in cloned array in this case.

[RealyUniqueName]

Maybe V8 could figure out the type of items in cloned array in this case.

Checked. It can't.

I've pushed array creation via new Array(length) in map. Because "native" Array.map() creates arrays with slow array access too.
So if one needs a lot of array mapping on js target, then Array.map() is the way to go. But if one needs faster array access, then var mapped = [for(i in original) func(i)]; is better.