-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
Performance problems #56
Comments
Cool that you're doing benchmarks! And sorry to hear that The parser that @branch14 was referring to is Ragel: https://en.wikipedia.org/wiki/Ragel As discussed, Ragel is pretty nice, so the following is nothing against Ragel. Having said that, the bottleneck for the long running test very likely isn't Java. Depending on the program, Java is within an order of magnitude of C or even faster, because it can optimize longer running processes on the spot. The TL;DR for speed is: Java programs are often slower to start (not so good for quick commandline tools, for example), but fast or faster for longer running apps. Therefore, there's likely an algorithmic issue going on (like we recently had). |
Ah, you've updated and are saying there's an |
If it's not obvious what the problem is (I wouldn't know atm, for example), my next step would be to debug this with https://visualvm.github.io/ to see what's happening while it's running. It'll show what functions are incurring time and memory. |
Another benchmark: @alexpdp7 wrote instaparse-cli which compiles to a binary. I just did a benchmark:
I wanted to see if a GraalVM binary makes the difference. But it's not a fair comparison so far because a) the JS runtime startup time is unknown and b) our parser also makes the transformation. I didn't tried a clojure script that parses without transformation. I would be something like:
(Don't know how to run that.) |
I wanted to try and reproduce the issue described by taking a real world 15k Org file. I took my personal gtd and reference files and the 200ok gtd file - together they are 16k LOC:
Running this with Java:
Granted, 18s is not fast in any stretch of the imagination - and it used > 4 cores in parallel, so on a single core it would have taken 85s. 85s is super slow and unacceptable to use for something like organice, of course. However, it doesn't crash and burn. Running it with JS shows a similar result:
JS does not run as much code in parallel, hence resulting in a longer real world compute time. Also inacceptable for organice, but also does not crash and burn. So, my first take-away is:
Can you produce an anonymized version of your Org file so that It can try and reproduce the issue with that? |
Some more benchmarks in the REPL: (time
(->
(slurp "test/org_parser/fixtures/minimal.org")
parser/parse)) Running this multiple times yields:
Or in other words, the actual time required to parse the minimal example is just fine. Taking my 16k LOC real world file: (time
(def a
(-> (slurp "real_world.org")
(parser/parse)))) NB: Using a tmp In multiple runs, this yields an elapsed time of:
20s is certainly too long just for parsing. Now, compare that with just the transformation step for the 16k LOC file: (time (def b (transform/transform a))) This yields:
Making it clear where the bottleneck is for my example: The parsing step. |
@schoettl The instaparse-cli benchmark you did in #56 (comment), does this use the same Org file as the benchmark that does OOM with org-parser? If I understand it right, instaparse-cli requires 'just' 4s and org-parser runs into OOM? If so, then this behaviour is again different than for my test. Because that would indicate that just parsing is relatively cheap on your file, but transforming it is too costly. To nail this down further, it would be good to have a common test file. And maybe you could run the parser and and transformer separately in the repl like I did in #56 (comment). I'm happy to help with this step, if it's unclear how to do, of course! |
How about this sample org file: sample.org: README.org ~/projects/organice/README.org ~/projects/organice/sample.org
cat $^ > $@
It's only
lines. But it uses a lot of org features we want to test. I tried to anonymize an org file with @munen Would you compare these results to yours:
EDIT: using latest master of both organice and org-parser. |
340ms for parsing isn't spectacular, but wouldn't be the end of the world. However, for you it's 10x the amount of time. That's already a dealbreaker. I can't imagine my machine being 10x faster than yours, but I ran this on a If I
Now we have a sample file on which we can do performance analysis. |
I'm afraid that some more digging didn't yield anything nicely actionable, yet. It looks like it requires more digging into instaparse to find the bottleneck. |
@branch14 Imo it would be best to continue in a pairing session here. |
One option to tackle this is to do binary search in the EBNF grammar. |
Preliminary finding: Parsing of the Some notes on that:
|
Hm, almost anything is
The order doesn't seem bad. E.g. If |
On the two-line sample file, org-parser takes
lein run
which is probably mainly compilation and runtime initialization.
On a 31,000 line real-world org file, it takes a maximum of CPU, 2GB of memory and at least 15 minutes (not yet finished).
(
lein run
crashed after 15 minutes withOutOfMemoryError
.)I haven't tried the JS version yet but I guess on a Smartphone it will only work well for files with a few hundred lines.
I'm a bit surprised because our parse trees should all be pretty flat.
@branch14 how was this language-independent state-machine parser generator called again? I guess a compiled binary compiler would be significantly faster.
The text was updated successfully, but these errors were encountered: