HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.18k stars 655 forks source link

Inefficient Map AST #5727

Closed ncannasse closed 4 years ago

ncannasse commented 8 years ago

Trying to pinpoint why HL allocate that much memory, I found the following:

class Main {

    var a : Map<Int,Main>;

    function foo(key:Int) {
        return a.get(key);
    }

    static function main() {
    }

}

Produces the following AST:

    function foo[Function:key : Int -> Null<Main>]
        [Arg:Int] [Local key(149):Int]
        [Block:Dynamic]
            [Return:Dynamic]
                [Block:Null<Main>]
                    [Var this1(152):IMap<Int, Main>]
                        [Field:Map<Int, Main>]
                            [Const:Main] this
                            [FInstance:Map<Int, Main>]
                                Main
                                a
                    [Call:Null<Main>]
                        [Field:key : Int -> Null<Main>]
                            [Cast:haxe.ds.IntMap<Main>] [Local this1(152):IMap<Int, Main>]
                            [FInstance:key : Int -> Null<Main>]
                                haxe.ds.IntMap<Main>
                                get
                        [Local key(149):Int]

What it does is first cast the Map<Int,Main> to IMap which is an interface, then cast it back (unsafely) to IntMap. If we could avoid these cast forth and back to interfaces, I'm sure all static type systems will be happy since it has some cost because the object value and interface value are two different things.

Simn commented 8 years ago

I think we should just throw out @:multiType and think of something else. It has major AST-issues with inlining anyway because the abstract disappears and the compiler doesn't know which specialization to select anymore.

ncannasse commented 8 years ago

@Simn any suggestion for a replacement? If it's only used by Map, we could hardcode the type selection in the compiler for now.

Gama11 commented 8 years ago

It's used by libraries: OpenFL's Dictionary, Flixel's FlxSignal, hxsignal, etc...

Simn commented 8 years ago

Maybe we could lose IMap itself by making Map into a @:coreType and take it from there. I'll have to investigate that...

nadako commented 8 years ago

Just so you know, this also generates the following on C#:

public virtual global::Main foo(int key) {
    return ((global::Main) ((((global::haxe.ds.IntMap<object>) (global::haxe.ds.IntMap<object>.__hx_cast<object>(((global::haxe.ds.IntMap) (((global::haxe.IMap<int, object>) (this.a) )) ))) ).@get(key)).@value) );
}

while direct IntMap usage generates much nicer code:

public virtual global::Main foo(int key) {
    return ((global::Main) ((this.a.@get(key)).@value) );
}
ncannasse commented 4 years ago

HL compilation does take care of removing these unnecessary casts, but still the compiler could output nicer code. Keeping open so it's linked to Map redesign (one day)

Simn commented 4 years ago

This is a duplicate of another issue that I'm too lazy to scroll back to right now.