haxeui / haxeui-core

The core library of the HaxeUI framework
http://haxeui.org
MIT License
346 stars 72 forks source link

scanClassPath() slows down compile times *a lot* #273

Closed Gama11 closed 5 years ago

Gama11 commented 5 years ago

I noticed that adding haxeui-core and haxeui-flixel to an empty flixel projects (just adding the usage sample code from haxeui-flixel's readme, using latest versions from GitHub) slows compile times a lot - a Neko compilation seems to take about twice as long (15 seconds instead of 7).

According to lime test neko --times -D macro-times -D eval-times, this all comes from scanClassPath():

name                     | time(s) |   % |  p% |      # | info
---------------------------------------------------------
macro                    |   8.105 |  50 |  50 | 201692 |
  execution              |   7.681 |  48 |  95 | 200775 |
    scanClassPath        |   7.048 |  44 |  92 |      3 | haxe.ui.macros.MacroHelpers
    readBytes            |   0.115 |   1 |   1 |      5 | sys.io._Process.Stdout
    endsWith             |   0.084 |   1 |   1 | 110073 | StringTools
    startsWith           |   0.072 |   0 |   1 |  67648 | StringTools
    ...

I haven't really checked why haxeui needs to scan the classpaths, but I'm assuming there's a good reason and it can't be avoided. However, this still seems a bit excessive, considering it does this three times in a single compilation (see # - 3 in the table). Couldn't there be some caching so at least only the first usage of scanClassPath() is slow?

Without -D eval-times, you can see the three macros making the calls are as follows:

macro                |   7.995 |  49 |  49 |   1226 |
  execution          |   7.571 |  47 |  95 |    309 |
    processBackend   |   2.450 |  15 |  32 |      1 | haxe.ui.macros.BackendMacros
    processModules   |   2.418 |  15 |  32 |      1 | haxe.ui.macros.ModuleMacros
    processNative    |   2.345 |  14 |  31 |      1 | haxe.ui.macros.NativeMacros

I also found some "skip patterns" in the sources, which suggests this has come up before:

https://github.com/haxeui/haxeui-core/blob/cd8b68a3d876e756f16640000f67f78b62d74cb4/haxe/ui/macros/MacroHelpers.hx#L11-L17

However, most of these wouldn't work for me since I have everything installed from GitHub with haxelib dev. So in my case the directory is actually <something>/Haxelibs/openfl, which means this is a very fragile solution. Also, Flixel isn't part of the list to begin with.

ianharrigan commented 5 years ago

Yeah, this is something that needs to be looked at for sure. The reasons it scans the classpaths is its looking for module.xml and native.xml - these can exist multiple times, in multiple projects at any package depth.

I agree this should probably be improved though, and im sure something can be done.

Cheers, Ian

ianharrigan commented 5 years ago

Any chance you could zip up your project and add it to the log btw? Will be easier to use as a template to work this out.

Gama11 commented 5 years ago

There's not a lot to it, so a .zip might be a bit overkill. You really just need these two files:

Project.xml:

<?xml version="1.0" encoding="utf-8"?>
<project>
    <app title="TestProject" file="TestProject" main="Main" version="0.0.1" company="HaxeFlixel" />
    <window width="640" height="480" fps="60" background="#000000" hardware="true" vsync="true" />

    <set name="BUILD_DIR" value="export" />
    <classpath name="source" />

    <haxelib name="flixel" />
    <haxelib name="haxeui-core" />
    <haxelib name="haxeui-flixel" />
</project>

source/Main.hx:

import flixel.FlxGame;
import flixel.FlxState;
import flixel.group.FlxGroup;
import openfl.display.Sprite;

class Main extends Sprite {
    public function new() {
        super();
        addChild(new FlxGame(640, 480, PlayState));
    }
}

class PlayState extends FlxState {
    override function create() {
        var myContainer = new FlxGroup();
        haxe.ui.Toolkit.init({container: myContainer});
        add(myContainer);
    }
}

Commenting out Toolkit.init() fixes the compile times. Due to the skip patterns, the issue is probably much less noticable if you have Lime and OpenFL from Haxelib.

ianharrigan commented 5 years ago

Can i get a print out of you classpaths Context.getClassPath()? Im taking a look at this and think it can be done much better (smarter exclusions and caching) - im also (at some point soon) going to refactor all of the macro stuff (using reification) so it think that will help with the actual macro speed also.

Gama11 commented 5 years ago

[export/neko/haxe/,source/,C:/Users/Jens/Haxelibs/haxeui-flixel/,C:/Users/Jens/Haxelibs/hscript/,C:/Users/Jens/Haxelibs/haxeui-core/,C:\Users\Jens\Haxelibs\hscript\,C:/Users/Jens/Haxelibs/lime/src/,C:/Users/Jens/Haxelibs/openfl/src/,C:/Users/Jens/Haxelibs/flixel/,C:\Users\Jens\GitHub\haxe\extraLibs/,,C:\Users\Jens\GitHub\haxe\std/neko/_std/,C:\Users\Jens\GitHub\haxe\std/]

ianharrigan commented 5 years ago

Thanks,

If you get a moment at some point, would you be able to test this out in MacroHelpers.hx, basically, replace the existing scanClassPath this this:

    private static var classPathCache:Array<String> = null;
    private static var topLevelClassPathExceptions:Array<EReg> = [
        new EReg("^build\\/", "gm"),
        new EReg("^export\\/", "gm"),
        new EReg("\\/openfl\\/.*src.?$", "gm"),
        new EReg("\\/actuate\\/.*src.?$", "gm"),
        new EReg("\\/lime\\/.*src.?$", "gm"),
        new EReg("\\/hscript", "gm"),
        new EReg("\\/flixel", "gm"),
        new EReg("\\/haxe\\/std", "gm"),
    ];
    private static var secondaryClassPathExceptions:Array<EReg> = [
        new EReg("\\/haxeui-core\\/cli.?$", "gm"),
        new EReg("\\/haxeui-core\\/haxe\\/ui.*\\w+", "gm"),
        new EReg("\\/haxeui-\\w+\\/haxe\\/ui\\/backend\\/\\w+", "gm"),
    ];

    private static function buildClassPathCache() {
        if (classPathCache != null) {
            return;
        }

        classPathCache = [];
        var paths:Array<String> = Context.getClassPath();
        for (path in paths) {
            path = StringTools.trim(path);
            path = Path.normalize(path);
            var exclude = false;
            for (r in topLevelClassPathExceptions) {
                if (r.match(path) == true) {
                    exclude = true;
                    break;
                }
            }
            if (exclude == true) {
                continue;
            }
            cacheClassPathEntries(path, classPathCache);
        }
    }

    private static function cacheClassPathEntries(path, array) {
        path = StringTools.trim(path);
        if (path.length == 0) {
            return;
        }
        path = Path.normalize(path);

        var exclude = false;
        for (r in secondaryClassPathExceptions) {
            if (r.match(path) == true) {
                exclude = true;
                break;
            }
        }
        if (exclude == true) {
            return;
        }

        var contents = sys.FileSystem.readDirectory(path);
        for (item in contents) {
            item = StringTools.trim(item);
            if (item.length == 0) {
                continue;
            }
            var fullPath = Path.normalize(path + "/" + item);
            if (sys.FileSystem.exists(fullPath) == false) {
                continue;
            }
            var isDir = sys.FileSystem.isDirectory(fullPath);
            if (isDir == true && StringTools.startsWith(item, ".") == false) {
                if (exclude == false) {
                    cacheClassPathEntries(fullPath, array);
                }
            } else if (isDir == false) {
                array.push(fullPath);
            }
        }

    }

    public static function scanClassPath2(processFileFn:String->Bool, searchCriteria:Array<String> = null) {
        buildClassPathCache();
        for (fullPath in classPathCache) {
            var parts = fullPath.split("/");
            var fileName = parts[parts.length - 1];
            if (searchCriteria == null) {
                processFileFn(fullPath);
            } else {
                var found:Bool = false;
                for (s in searchCriteria) {
                    if (StringTools.startsWith(fileName, s)) {
                        found = true;
                        break;
                    }
                }
                if (found == true) {
                    processFileFn(fullPath);
                }
            }
        }
    }

    // ------------------------------- ORIGINAL SCAN HERE
    public static function scanClassPath(processFileFn:String->Bool, searchCriteria:Array<String> = null, skipHidden:Bool = true) {
        buildClassPathCache();

        scanClassPath2(processFileFn, searchCriteria); //  TODO: temp for now, to make sure its all working
    }

Would be interested to see what the results you get are.

For me scanClassPath doesnt even show up in the times now - there is still alot to do wrt to the macros in general, i think the reification will be a really nice refactor (as well as speed just general ease of maintenance)

Many thanks, Ian

Gama11 commented 5 years ago

Yeah, that seems like a big improvement:

macro                |   0.901 |   9 |   9 |   1176 | 
  execution          |   0.491 |   5 |  54 |    287 | 
    build            |   0.127 |   1 |  26 |     10 | openfl._internal.macros.ShaderMacro
    buildGitSHA      |   0.115 |   1 |  24 |      1 | flixel.system.macros.FlxGitSHA
    processModules   |   0.100 |   1 |  20 |      1 | haxe.ui.macros.ModuleMacros
    build            |   0.030 |   0 |   6 |     63 | lime._internal.macros.EventMacro
    addClonable      |   0.025 |   0 |   5 |     45 | haxe.ui.macros.Macros
    buildStyles      |   0.024 |   0 |   5 |     45 | haxe.ui.macros.Macros
    embedBitmap      |   0.022 |   0 |   5 |     36 | openfl._internal.macros.AssetsMacro
    processBackend   |   0.016 |   0 |   3 |      1 | haxe.ui.macros.BackendMacros
    buildMap         |   0.011 |   0 |   2 |      5 | flixel.system.macros.FlxMacroUtil
    buildBindings    |   0.009 |   0 |   2 |     44 | haxe.ui.macros.Macros
    build            |   0.003 |   0 |   1 |      7 | lime._internal.macros.HTTPRequestMacro
    embedBytes       |   0.002 |   0 |   0 |      5 | lime._internal.macros.AssetsMacro
    run              |   0.002 |   0 |   0 |      1 | flixel.system.macros.FlxDefines

With -D eval-times:

macro                       |   1.222 |  12 |  12 |  19109 | 
  execution                 |   0.777 |   8 |  64 |  18220 | 
    toplevel                |   0.254 |   3 |  33 |    270 | 
    readBytes               |   0.097 |   1 |  12 |      5 | sys.io._Process.Stdout
    hx                      |   0.092 |   1 |  12 |     14 | C:/Users/Jens/Haxelibs/openfl/src/openfl/_internal/macros/ShaderMacro
    getModule               |   0.072 |   1 |   9 |     96 | haxe.macro.Context
    getBuildFields          |   0.046 |   0 |   6 |    218 | haxe.macro.Context
    build                   |   0.034 |   0 |   4 |     20 | openfl._internal.macros.ShaderMacro
    new                     |   0.028 |   0 |   4 |      2 | sys.io.Process
    resolvePath             |   0.016 |   0 |   2 |     39 | haxe.macro.Context
    defineType              |   0.013 |   0 |   2 |     28 | haxe.macro.Context
    hasInterface            |   0.012 |   0 |   2 |     45 | haxe.ui.macros.MacroHelpers
    embedData               |   0.011 |   0 |   1 |     36 | openfl._internal.macros.AssetsMacro
    buildClassPathCache     |   0.009 |   0 |   1 |      6 | haxe.ui.macros.MacroHelpers
[...]

It still seems a bit concerning that this relies on hardcoding particular libs, I'd imagine people using heaps or kha might run into similar issues?

Btw, it'd be much nicer to use regex literals here (~/\/openfl\/.*src.?$/gm). ;) Less escaping, and you get syntax highlighting in VSCode.

ianharrigan commented 5 years ago

Great, yeah, what im thinking is that each backend can also have like a .exclusions file in the root, and that can add framework specific regexps there, but first i just wanted to make sure that the changes do indeed make an overall difference... thanks!

Ian

ianharrigan commented 5 years ago

Can this be considered closed now?