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

Use keyword searching to improve parsing speeds #5103

Conversation

APickledWalrus
Copy link
Member

Description

This PR improves parsing speeds by looking for keywords Skript can search for in input.
This applies to any pattern where you have a LiteralPatternElement immediately after a TypePatternElement. One notable example of this is EffChange. For example, with a pattern like set %objects% to %objects%, Skript can now find what characters represent the first %objects% far faster. This is because, of course, Skript is now search by every occurrence of to instead of going character by character. I haven't tested it, but I believe this should also improve Skript's ability to handle very long lines.

On my personal dev server, where I use the scripts from #4605, I noted about a 14.5% improvement (from 129.4 ms avg to 113 ms avg.)

I also tested using a far larger script collection. They are slightly outdated, so some syntax did error (which would increase parsing time). Without these changes, it took about 36 seconds on average to load. With these changes, it only took 7. This is over a 400% increase - quite awesome!

I believe the changes are stable enough - I compared all of the errors from the larger script collection to ensure they remained the same.


Target Minecraft Versions: Any
Requirements: N/A
Related Issues:

it causes problems with optional patterns following the literal. example:
`%dates% formatted [human-readable]`
So, instead of searching for ` formatted `, we are now searching for ` formatted`
@APickledWalrus APickledWalrus added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Sep 16, 2022
@TheBentoBox
Copy link
Member

TheBentoBox commented Sep 17, 2022

Really awesome update. Tested out the nightly, here are my findings. Will start with the issues.


  1. Double spaces within a line now error. This didn't give an error previously. I don't even mind this being an error tbh since it was a mistake, but maybe the message should be clearer if you choose to leave it as-is.

image

  1. Some custom syntaxes with skript-mirror are broken. Most of them are fine, but a few randomly seem to not work. I'll give a few examples in case they reveal separate issues:

Syntax 1:
image
image

Syntax 2:
image
image

Syntax 3:
image
image

One or two more misc ones, but you get the picture. It's only this handful of them though, the rest all load fine (and I have many). I tried an extra restart, and the same issue occurs for the same syntaxes, so it's consistent at least.


As for the good: even with the decent handful of errors caused by those above issues, my script load times dropped from ~40 seconds to ~28 seconds! About a 30% TOTAL time savings. Pretty awesome!

@TheBentoBox
Copy link
Member

Figured it out: It's case-sensitivity. It seems that expressions are registered in lowercase regardless of how they're cased in their declaration (not sure if that's a skript-mirror thing or Skript itself), but the literal keyword searching is case-sensitive.

e.g., For the mcMMO example, I have "XP" capitalized in the expression declaration, but when I use it, I have to do "xp", otherwise it errors. Same for the others ("CTF" -> "ctf" and "Impaling" -> "impaling").

Seems like you just need a lowercase conversion somewhere and it should be all good

- Fix an issue where syntax searching was case sensitive
- Fix an issue where extraneous space handling no longer worked
@TheBentoBox
Copy link
Member

Never commented here but just want to note that the above fixes do work and I get no errors on my test server using this version. I've been using it for two weeks and have noticed no change in behavior in anything I've interacted with.

@APickledWalrus APickledWalrus added the 2.7 Targeting a 2.7.X version release label Jan 20, 2023
@TheLimeGlass TheLimeGlass merged commit 76c462b into SkriptLang:master Jan 25, 2023
@APickledWalrus APickledWalrus deleted the enhancement/expression-searching-improvements branch September 6, 2023 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.7 Targeting a 2.7.X version release enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants