SkriptLang / Skript

Skript is a Spigot plugin which allows server admins to customize their server easily, but without the hassle of programming a plugin or asking/paying someone to program a plugin for them.
https://docs.skriptlang.org
GNU General Public License v3.0
1.08k stars 373 forks source link

Script load times increased since dev28 #636

Closed TrademarkTM closed 7 years ago

TrademarkTM commented 7 years ago

Hello again Bensku. I've noticed a big problem in the new 28c version of Skript. My scripts are taking way more time to load.

28c and 29

[16:20:35] [Server thread/INFO]: loading 'ItemXP.sk'
[16:20:35] [Server thread/INFO]: loading trigger 'on load'
> 2 seconds here?
[16:20:37] [Server thread/INFO]: loading trigger 'on break'
[16:20:37] [Server thread/INFO]: loading trigger 'on rightclick holding a pickaxe'
[16:20:37] [Server thread/INFO]: loading trigger 'on place'
[16:20:37] [Server thread/INFO]: loading trigger 'on block damage'
[16:20:37] [Server thread/INFO]: loading trigger 'on command "gm"'
[16:20:38] [Server thread/INFO]: loaded 6 triggers and 0 commands from 'ItemXP.sk'

Took ~ 3 seconds to load.

27

[16:25:35] [Server thread/INFO]: loading 'ItemXP.sk'
[16:25:35] [Server thread/INFO]: loading 'ItemXP.sk'
[16:25:35] [Server thread/INFO]: loading trigger 'on load'
[16:25:35] [Server thread/INFO]: loading trigger 'on break'
[16:25:35] [Server thread/INFO]: loading trigger 'on rightclick holding a pickaxe'
[16:25:35] [Server thread/INFO]: loading trigger 'on place'
[16:25:35] [Server thread/INFO]: loading trigger 'on block damage'
[16:25:35] [Server thread/INFO]: loading trigger 'on command "gm"'
[16:25:35] [Server thread/INFO]: loaded 6 triggers and 0 commands from 'ItemXP.sk'

Less than 1 second to load.

My script is this one: https://hastebin.com/qehorawava.vbs Hope you can fix it! Thanks!

aescraftbr commented 7 years ago

Kinda noticed that too. In 27 it took like 13seconds to load all my skripts, now it takes 20+ seconds.

netherstar commented 7 years ago

Confirm, on dev 27 it took 6 seconds to load this very simple skript.

on join:
    wait 2 ticks
    make player execute command "/sw join"
KorkugunuB commented 7 years ago

I have the same problem

Blueyescat commented 7 years ago

Takes 1 minute+ for my some skripts :)

bensku commented 7 years ago

Need to do some profiling to figure out what is causing this. I hope it is just some random mistake I made - if throwing vectors in causes it, I'll have interesting time trying to fix it.

bensku commented 7 years ago

@tuanjr For me it takes <1 second with dev25 and about same time with latest version. I need some test material, a script that:

To clarify: I'm not interested about, for now, any even potentially addon-related slowdowns with latest Skript. I will take look at them after I have done everything I can to get Skript itself running at least as fast or as slow as before.

From what I have seen now, SkriptParser's parse_i is hotspot and slow, but that is nothing new. It has been so for years...

Sashie commented 7 years ago

untitled that load time indeed!!

Sashie commented 7 years ago

you should add some custom timings to the parser to see how long it takes to load each effect/expression and a few timings to see whats happening in each syntax parse

bensku commented 7 years ago

I'll consider that once someone actually provides test scripts I asked for in previous post.

TrademarkTM commented 7 years ago

The first lines of code of my script are pure sk and are taking 2 seconds to load:

options:

    xp.maxlevel: 100

    xp.base.pickaxe: 200
    xp.increase.pickaxe: 0.2

    xp.blocks.pickaxe: stone-0.6;cobblestone-0.2;andesite-1;diorite-1;granite-1;diamond_ore-8;emerald_ore-100;lapis_lazuli_ore-10000

    xp.base.shovel: 200
    xp.increase.shovel: 0.15

    xp.blocks.shovel: dirt-0.3;sand-0.5;gravel-1.0;grass-0.4;red sand-0.6

on load:
    clear {config::*}
    set {config::base::pickaxe} to {@xp.base.pickaxe}
    set {config::increase::pickaxe} to {@xp.increase.pickaxe}
    loop split "{@xp.blocks.pickaxe}" at ";":
        set {config::blocks::pickaxe::%subtext of loop-value from characters 1 to first index of ""-"" in loop-value - 1 parsed as item%} to subtext of loop-value from characters first index of "-" in loop-value + 1 to length of loop-value
    set {config::base::shovel} to {@xp.base.pickaxe}
    set {config::increase::shovel} to {@xp.increase.pickaxe}
    loop split "{@xp.blocks.shovel}" at ";":
        set {config::blocks::shovel::%subtext of loop-value from characters 1 to first index of ""-"" in loop-value - 1 parsed as item%} to subtext of loop-value from characters first index of "-" in loop-value + 1 to length of loop-value

The full script was taking < 1s before.

Snow-Pyon commented 7 years ago

Can confirm, tested and it takes around 1.916 seconds with the dev28 whereas it's around 0.901 seconds with the dev25 and 0.6 seconds with the dev27.

bensku commented 7 years ago

Ok, this seems useful. I'll try to figure out what is wrong and how to fix it.

Also nice to know that dev27 actually improved performance by about 33% :)

bensku commented 7 years ago

I have disabled number to vector converter for next release to solve another issue. When it is out, please test if it helps with performance.

Sashie commented 7 years ago

It certainly didn't help make my test go any faster :c (compiled from source) One thing to note is that on multiple starts it would finish in a semi random amount of time between about 9m40s to 10m50s

bensku commented 7 years ago

I'm out of ideas. Will try something once I'm back home (in about week), but it might break stuff even more. Help would be appreciated...

andrewjunggg commented 7 years ago

dev25 took 4 seconds to reload my script, dev29 will crash my server (timed out) every time i reload it. and vector converter have an issue with RandomSk if i remembered well. Maybe you should check that too :wink:

Snow-Pyon commented 7 years ago

@nutella25 he will not change the code just because an addon which isn't supported anymore is conflicting with it, you can just enable the soft api exceptions option on your config.sk and problem solved.

Sam1370 commented 7 years ago

Someone change the title to "28c and 29 take way longer to load scripts."

Sam1370 commented 7 years ago

Nevermind, saying "increased" is better :P

Sam1370 commented 7 years ago

Should I keep using dev29 or use a version that's older?

TheBentoBox commented 7 years ago

If the load times aren't an issue for you there's no real reason to not use dev29. If you don't want to use dev28 or dev29 because of the load times though, I'd go back to dev25, since dev26-27 both had a number of issues, mainly with chat stuff due to the JSON chat support.

Sam1370 commented 7 years ago

@TheBentoBox I'm using json.sk (https://www.spigotmc.org/resources/json-sk.8851/). Does that count as json chat support because I need that. Also the load times are a real issue for me, my skripts in total take 1 minute and 14 seconds to load as of my last one. So if I do /skript reload scripts it crashes the server, so instead I have to restart the server (which isnt a problem since no one really joins on it) but it's still a real hassle :( EDIT: I used dev25 and it works great, skripts reload in 41 secs which I think isn't enough to make Spigot crash the server

Sashie commented 7 years ago

I can confirm that load times on dev25 are way faster..

665

(2 minutes down to 20 seconds for what i'm testing)

Sam1370 commented 7 years ago

@Sashie hmm, takes 45 secs down from about a minute for me. But that's just my skripts

Sam1370 commented 7 years ago

Still not fixed, @bensku you still working on it?

Snow-Pyon commented 7 years ago

@Sam1370 it isn't something that can be easily solved, since it relies on how Skript parses the syntaxes, be patient and wait for bensku to find a solution to this problem.

bensku commented 7 years ago

I'm not sure if I'll ever manage to fix this. That is, without disabling some (a lot) of Skript features. Buy yes still working on it.

Sam1370 commented 7 years ago

@Snow-Pyon Oh, thanks for the info :D @bensku mmm, okay, should I keep using dev25 then?

bensku commented 7 years ago

@Sam1370 It lacks some big fixes and new features. If that is not an issue, maybe.

Tristag commented 7 years ago

This is prob not the best suggestion, however, what if you just allow some features to be turned off/on through the config?

It seems they increased post-25 so basically one could turn off the features causing the load times while keeping ones that don't on, so it's at least something new :)

Anyways, good luck on this and it is immensely appreciated!

Snow-Pyon commented 7 years ago

This is prob not the best suggestion, however, what if you just allow some features to be turned off/on through the config?

Well, this is something that Mirreski wanted to do (look at the features.sk file) and I doubt it actually works but the system is still there so this could be possible.

Tristag commented 7 years ago

Maybe not my worst idea then lol

There are some bugs that I wish I could have fixed, however the exponential load times of the new versions make it impossible to run. I understand that it's really hard to fix all the bugs though so I won't complain, just my reasoning for the suggestion.

bensku commented 7 years ago

I have changed parser a bit, and will perform further changes either for next release or the release after that. While fixing a parser bug, I noticed that there are few relatively easy ways to improve parsing performance.

Tristag commented 7 years ago

That sounds awesome! Can't wait! Thanks a ton man!

bloggy commented 7 years ago

That's from July, still working on it?

bensku commented 7 years ago

I have still not received sufficient test case, which is a script which take 1+ minutes to parse with dev25, parses way slower with latest release and does not use addons. Do you have one, @bloggy?

Edit: I mean, I do have some test files, but they do not seem to parse much faster with dev25...

bloggy commented 7 years ago

I am sure that I have NO script that is pure skript without addons, but I wonder why u are unable to reproduce it? There must be something happened to the parser making it so slow?

EDIT:

But it isn't hard to make a skript that takes forever to reload with the latest skript version. Here is an example (this is pure skript btw): https://hastebin.com/tovajupamu.coffeescript (This takes 30 seconds to reload in v2.2-dev31c)

Just copy / paste the same events and stuff in the same file over and over again. The more lines it has, the longer the parser takes to do its job.

Reloading the skript in my link even kicks me out of my server (time out / connection lost).

Btw. copying the lines of that script again and putting them at the end of the same file and doing a reload -> will crash my server because skript is freezing it then.

EDIT 2:

Reloading the same script as linked will take 9 seconds to reload on skript v2.2-dev25.

For me, having 58 scripts, Skript is unusable until you fix the parser loading time. :-( Let me know if you need further information for helping with this.

bensku commented 7 years ago

30 seconds vs. 9 seconds is probably good enough. I can probably use your test script to figure out what is wrong, given enough time for debugging.

I guess that is what I will be doing next weekend. Wish me luck :)

bensku commented 7 years ago

Done, closing when next release is out. Uhh, that took a long.

TheBentoBox commented 7 years ago

The hero we need!

bloggy commented 7 years ago

Really? You fixed it so fast? :-) Mind sharing what the issue was? And when will you release the update? I really need it :-)

Thanks for your time and effort!

TheBentoBox commented 7 years ago

@bloggy It appears to have been the location from vector expression judging by commits. Additionally, since this was added as a milestone for 2.3, it seems this fix will be released when 2.3 is, which will happen once the issues on the 2.3 milestone list are resolved.

Snow-Pyon commented 7 years ago

@bloggy you're still able to get the build which is supposed to fix this with the nightly builds repository: https://github.com/bensku/skript-build

Be aware that these builds aren't stable at all.

bensku commented 7 years ago

Right now nightly builds are 100% broken due to other changes. Maybe tomorrow...

bloggy commented 7 years ago

Can you please tell us when you are planning on releasing the fixed version? Still unable to use Skript with 1.12 because of this bug. :-( Hope you can push an update for this.

Syst3ms commented 7 years ago

He fixed it in recent commits, you can check out the nightly build repo (linked above) to try these versions

TheBentoBox commented 7 years ago

@bloggy dev25 works completely fine with 1.12.0

bensku commented 7 years ago

I do not have time to work on Skript right now, hopefully next weekend or so changes this.

bloggy commented 7 years ago

@TheBentoBox for me it does not work 100% on 1.12.2 with your mentioned version: https://hastebin.com/omujaciteq.vbs

Also tried the latest nightly build but then all of my skript commands stop working (it always says: unknown command). But does not throw any errors.

TheBentoBox commented 7 years ago

@bloggy Are you on 1.12.0 or a newer one? (1.12.1/1.12.2)