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

Local optimizations take forever to complete on 24-player maps with 2 forces #700

Closed
ElusiveMori opened this issue Jul 31, 2018 · 4 comments

Comments

@ElusiveMori
Copy link

Steps to reproduce:

  1. Make a 24-player map like this:
    1

  2. Make 2 forces like this:
    2

Saving this map in WorldEdit produces this monstrosity of a function:
https://pastebin.com/JED2J7V3

If inlining is turned on, this gets turned into an even bigger monstrosity:
https://pastebin.com/r3kURDAY

That is 4 thousand lines of JASS code.

If you now turn on local optimizations, it takes forever for them to actually run, compared to 2-4 seconds on normal 12-player maps (or a 24-player map with no forces). If you remove these 2 forces, everything goes back to normal.

I don't know how this bug should be addressed, but it can be a major pain for maps with 24 players where 2 or more forces are involved. I like compiling with all optimizations enabled because even on my big project (15k lines of optimized jass emitted) it doesn't take too long to finish, with 90% of stdlib imported. But this function, it just can't regurgitate.

One of the safer options would be to just disable optimizations on the config function directly, since it only runs once per map and hardly needs optimization at all. I don't think anyone would suffer from it.

@Frotty
Copy link
Member

Frotty commented Jul 31, 2018

Saving this map in WorldEdit produces this monstrosity of a function:

What does it look like normally?

If you now turn on local optimizations, it takes forever for them to actually run, compared to 2-4 seconds on normal 12-player maps (or a 24-player map with no forces). If you remove these 2 forces, everything goes back to normal.

Just a big amount of lines shouldn't make local opts take exponentially longer - so probably another issue at hand here.

One of the safer options would be to just disable optimizations on the config function directly, since it only runs once per map and hardly needs optimization at all. I don't think anyone would suffer from it.

Seems kind of overkill - fixing this behavior should be sufficient.

@Frotty
Copy link
Member

Frotty commented Aug 2, 2018

Idk, did you highly exaggerate here?
Takes like 10-15 seconds instead of 5-10 for me.
Really not critical.

e: @Samuelmoriarty The minimal "workaround" for this seems to simply not inline the SetPlayerAllianceStateAllyBJ BJ. Pushed a fix to keep these things in blizzard.j. Please verify.

peq added a commit that referenced this issue Aug 8, 2018
not sure if the optimization is still correct with this change ...
@peq
Copy link
Collaborator

peq commented Aug 8, 2018

The problem is that TempMerger has a quadratic running time in the length of the function, since it always starts from beginning of the function after finding the first thing it can replace. Of course this is problematic for such a big function.

I created a branch with a change that should reduce the running time of the TempMerger, but I have to think about it a bit more. I am not sure if it is still correct this way ...

@Frotty
Copy link
Member

Frotty commented Jan 14, 2024

has been fixed by #1024

@Frotty Frotty closed this as completed Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants