Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[js] drastic slowdown in Type.enumEq with Haxe 4.0.0 preview 4 #7189

Closed
AlexHaxe opened this issue Jun 13, 2018 · 9 comments
Closed

[js] drastic slowdown in Type.enumEq with Haxe 4.0.0 preview 4 #7189

AlexHaxe opened this issue Jun 13, 2018 · 9 comments
Assignees
Labels
performance platform-javascript Everything related to JS / JavaScript regression
Milestone

Comments

@AlexHaxe
Copy link
Contributor

During tests with the latest Haxe 4 preview 4 and vscode-checkstyle I noticed a drastic slowdown in response times between introducing a checkstyle violation in code and the appearance of a new diagnostic message in problems view. It took like 10-15 seconds, where it should only take a brief moment.
@Gama11 pointed me in the direction of -D js-enums-as-arrays and #6350.
Setting -D js-enums-as-arrays restores speed to previous levels.

I tried to build a minimalised demo, and I came up with this: http://try-haxe.mrcdk.com/#5D413
Try running it with Haxe 3 and Haxe 4 and see the difference in speed:

  • ~0.003s Haxe 3 with Chrome
  • ~0.4s Haxe 4 with Chrome
  • ~2.0s Haxe 4 with Firefox

Results with nodejs on command line are comparable to Chrome.

@AlexHaxe AlexHaxe changed the title [JS] drastic slowdown in Type.enumEq with Haxe 4.0.0 preview 4 [js] drastic slowdown in Type.enumEq with Haxe 4.0.0 preview 4 Jun 13, 2018
@nadako
Copy link
Member

nadako commented Jun 13, 2018

I'm pretty sure that this is related to the Reflect.fields call which is now there. We should instead generate a native js for loop.

@nadako nadako self-assigned this Jun 13, 2018
@nadako nadako added platform-javascript Everything related to JS / JavaScript regression performance labels Jun 13, 2018
@nadako nadako added this to the Release 4.0 milestone Jun 13, 2018
@nadako
Copy link
Member

nadako commented Jun 13, 2018

Or maybe we should get the __params__ of the enum constructor and iterate over it, I'll check it out later.

@nadako
Copy link
Member

nadako commented Jun 13, 2018

@AlexHaxe BTW you shouldn't be using Type.enumEq on enum abstracts :)

@nadako
Copy link
Member

nadako commented Jun 13, 2018

@Simn actually, this is weird: i'm getting a diagnostic message Constraint check failure for enumEq.T TestEnum should be EnumValue, but the compilation works.

@nadako
Copy link
Member

nadako commented Jun 13, 2018

@Simn I think the problem is that the diagnostic comes from the @:coreApi Type.enumEq version which has a EnumValue constraint on its T, while the JS version doesn't have any constraints. We should check the constraints in the @:coreApi validation?

@AlexHaxe
Copy link
Contributor Author

I don't think checkstyle uses Type.enumEq on enum abstracts, but that was the smallest example I could come up with.

A full example would be to:

  • install git versions of hxparse, haxeparser, hxnodejs, tokentree and haxe-checkstyle
  • install haxelib versions of hxargs, compiletime
  • compile to javascript using haxe buildJS.hxml in haxe-checkstyle
  • run node haxecheckstyle.js -s src/checkstyle/Main.hx - scans just one file
  • compile again using haxe buildJS.hxml -D js-enums-as-arrays
  • run node haxecheckstyle.js -s src/checkstyle/Main.hx
  • observe difference between first and second run (~10s and ~0.5s). I used time in front of my runs, but that probably only works on Linux (and maybe MacOSX)

@nadako
Copy link
Member

nadako commented Jun 13, 2018

@AlexHaxe I just pushed an optimization, but i never measured it, could you try it maybe? :) See f0e026e

@AlexHaxe
Copy link
Contributor Author

The sample code doesn't compile any more (but I guess that's intentional):

Test.hx:8: characters 9-20 : Constraint check failure for enumEq.T
Test.hx:8: characters 9-20 : TestEnum should be EnumValu

checkstyle on the other hand compiles fine and I see no difference whether I compile with -D js-enums-as-arrays or without - there are slight variations in timings with every run, but we are within the same range and the first non-zero digit is equal for most of my runs.
So it seems like your optimisation is working.

@nadako
Copy link
Member

nadako commented Jun 13, 2018

Good! Regarding the @:coreApi I guess I'll make another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance platform-javascript Everything related to JS / JavaScript regression
Projects
None yet
Development

No branches or pull requests

3 participants