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

ERROR java.lang.RuntimeException: Method code too large! #235

Closed
elydpg opened this issue May 28, 2016 · 13 comments
Closed

ERROR java.lang.RuntimeException: Method code too large! #235

elydpg opened this issue May 28, 2016 · 13 comments
Assignees
Labels

Comments

@elydpg
Copy link
Contributor

elydpg commented May 28, 2016

Changed the key of my score, which caused some + characters to be added. Doesn't seem to like the new length, as the server is throwing this error whenever I try to play it:

[27713] ERROR java.lang.RuntimeException: Method code too large!, compiling:(NO_SOURCE_PATH:0:0)

This happens when loading files into the REPL as well.

in 1.0.0rc14 it loads, however that was back when [27713] ERROR Read timed out was an issue. I wonder if that had anything to do with it?

@daveyarwood
Copy link
Member

Hmm... this looks like a limitation of the Java Virtual Machine. In googling the error message, it appears that the bytecode for a Java class can be at most 64 KB. So the questions are, 1) where exactly is all this byte code being generated? and 2) what can we do to work around this limitation?

My theory is that this has to do with the parsing phase... can you try running alda parse -f /path/to/your/score.alda and see if the error happens then?

If that succeeds, then try alda parse --map -f /path/to/your/score.alda which will parse AND evaluate the score, and see if that fails.

@elydpg
Copy link
Contributor Author

elydpg commented May 28, 2016

alda parse -f succeeds, but alda parse --map -f fails.

@daveyarwood
Copy link
Member

That's good to know -- so the issue must be something to do with huge Java classes being generated when the Clojure code gets eval'd.

Can you post the output of alda parse? I can play around with it and see if I can narrow down where the issue is.

@elydpg
Copy link
Contributor Author

elydpg commented May 29, 2016

Any way I could make this collapsable? It's really long...

(alda.lisp/score
 (tempo! 160)
 (alda.lisp/part
  {:names ["midi-synth-voice"]}
  (alda.lisp/voices
   (alda.lisp/voice 1 (vol 175/2))
   (alda.lisp/voice 2 (vol 200/3))
   (alda.lisp/voice 3 (vol 200/3))
   (alda.lisp/voice 4 (vol 200/3))
   (alda.lisp/voice
...

EDIT: truncated and pasted here: http://pastebin.com/36Tn9XXm - Dave

@daveyarwood
Copy link
Member

I tried evaluating this huge Clojure expression and got the same "Method code too large!" error after running eval on the code. It appears that when using eval on a Clojure expression, the more arguments the S-expression has, the larger the generated Java byte code.

I found this relevant discussion on the Clojure mailing list about evaluating machine-generated code, which is exactly what Alda does. The moral of the story seems to be that it's not a good idea to generate huge amounts of code and then evaluate it with eval because it results in compiling a huge Java class that can't be run because the method code is too large. The alternative is to generate a small amount of code that can do large amounts of stuff dynamically / lazily, which is something Clojure is good at.

@elyisgreat This limitation sucks, but I'm glad you ran into it, because it's going to force us to optimize the way the Alda parse/eval process works. This will be a fairly involved change... I hope you don't mind if this issue stays open for a while.

I think the solution is going to be moving away from eval and having the parser emit an optimized sequence of events instead of Clojure code. I think we can do this and still provide a Clojure DSL, i.e. if you're writing a Clojure program and you write code using the Alda Clojure DSL, it will still work, it's just that the Alda server's parse/eval process will not emit and eval Clojure code, it will emit data in a more optimized form and use it to generate the score without using eval.

#133 should help as a workaround for this issue, too, although ideally I'd like to solve it so that there isn't a limitation to how long an Alda score can be.

@daveyarwood
Copy link
Member

I did a little testing and it appears that the size limit of an Alda score, with our current eval-based system, is (very) roughly 7000 characters. This is about 3 times the length of the Bach cello suite no. 1 example

@daveyarwood
Copy link
Member

I have a work-in-progress branch where I implemented the non-eval-based solution I mentioned above. It can handle parsing a large score (I made one by copy-pasting the Bach cello suite example three times) without running into the "Method code too large!" error, but unfortunately, performance is worse than what we have currently. For example, it takes 3 seconds to process the Bach cello suite example, whereas what we have currently does it in 2 seconds.

I think this approach might still be feasible if we can optimize the parser. (related: #208)

@daveyarwood
Copy link
Member

daveyarwood commented Jun 9, 2016

Doing some more experimentation using 3 versions of Alda...

  1. Current release version (1.0.0-rc18)
  2. A branch where @aengelberg and I are cooking up some significant parser optimizations ( Parser optimizations #238 )
  3. A branch built on top of that branch, with the parser optimizations and the approach I described above, where we emit pre-evaluated data instead of Clojure code to be eval'd.

Average time to parse/evaluate the Bach cello suite example:

  1. About 2 seconds.
  2. About 1.8 seconds.
  3. About 2.9 seconds.

So, even with the parser optimizations*, trying to work around the "Method code too large!" error by pre-compiling the score instead of generating Clojure code and evaluating it like we're doing currently, causes a substantial performance hit.

This is disappointing. I'm not willing to make a change that will make the parse/eval process even slower than it already is, so back to the drawing board I guess. I'm going to try still generating the Clojure code, but trying to break it up into smaller chunks and eval-ing them separately as part of the same score.

*As an aside, it's surprising that the parsing optimizations only save us 0.2 seconds here, whereas in the benchmarking tests, they save about 0.5-0.6 seconds. I wonder if the server is causing additional overhead.

@daveyarwood
Copy link
Member

After exploring this a little more, it turns out that the performance issues I thought I was seeing above are actually largely due to the Alda server pretty-printing the parse responses. (I was testing by running alda parse -f examples/bach_cello_suite_no_1.alda.) I think moving away from eval might actually be faster. Will play around with this a little more (removing pretty-printing from the equation to get more accurate benchmark results) when I have time.

Ideally, I'd like for alda parse to still return Clojure code, but under the hood when you play something (i.e. with alda play), it won't generate code and eval it, it will just skip to doing the work. That should be doable.

@daveyarwood
Copy link
Member

daveyarwood commented Jun 18, 2016

It seems like eval is still slightly faster*, which is kinda weird, but I think the difference might be negligible enough to make it worth fixing the "Method code too large!" problem.

*The Bach example takes 850-900 ms to parse to Clojure code and then eval, vs. 1150-1200 ms to have the parser evaluate events directly without them having an intermediate form. If I don't print the results (i.e. pipe them to /dev/null), it brings it down to 750 ms vs. 1000 ms. This is all including the parser optimizations in #238.

I think I'll prepare a release of Alda that includes both the parser optimizations and the change to not use eval. Comparing those changes bundled together vs. the current release (1.0.0-rc18), it's a lot faster (1000 ms is a lot better than 2000-3000), and will fix the "Method code too large!" problem.

@daveyarwood
Copy link
Member

Fixed in 1.0.0-rc19.

@daveyarwood
Copy link
Member

Oops... looks like a bug snuck in. Sorry! Will fix ASAP.

@daveyarwood
Copy link
Member

OK, fixed in 1.0.0-rc20 😜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants