Worksheet plugin should allow users to specify JVM args such as maximum heap size

[hunam]

Hi Iulian and Mirco!

Looking at ProgramExecutor.scala, it seems that the process is launched with no configurable arguments.

I'm currently prototyping some text processing code using worksheets, and keep running into OutOfMemoryErrors, with no remedy.

Adding a preference setting for JVM arguments used by worksheet processes would solve the issue, and is also consistent with how settings are specified for the compiler.

As a quick and dirty alternative solution, a system property (e.g. 'org.scalaide.worksheet.jvmargs') can be supported with minimal effort.

[dotta]

Hi Nadav :)

Good point.

Adding a user-configurable setting for the JVM arguments looks like a good idea to me. Having a system property would also be viable, but IMO it's less clean and harder to discover for users.

Any chance you'd be interested in contributing it?

[dragos]

@hunam, good to hear from you!

[hunam]

Gladly, I'm on it!

[dotta]

@hunam Awesome!

[nadavwr]

I'm just about done.

The preferences page was having some layout problems:

The easiest thing to do was to change the preference page to grid layout:

class Preferences extends FieldEditorPreferencePage(FieldEditorPreferencePage.GRID) //...

This is how it now looks: Much better, but you will notice how the text fields have become quite narrow. This makes it difficult for editing or perusal of VM arguments.

Other than introducing code for custom layout on the preferences page (beyond the scope of this change), the easiest thing to do could be to trim the cutoff preference description to something shorter like "Output character limit per statement". However, I've left things as they are in the latter screenshot for now.

As for the change itself -- here is the output from a sample heapkilling worksheet with no VM arguments specified:

object HeapKiller {
 (1 to 1e8.toInt).toVector.length                 //> java.lang.OutOfMemoryError: Java heap space
                                                  //|   at scala.collection.immutable.VectorPointer$class.gotoNextBlockStartWrit
                                                  //| able(Vector.scala:886)
                                                  //|   at scala.collection.immutable.VectorBuilder.gotoNextBlockStartWritable(V
                                                  //| ector.scala:692)
                                                  //|   at scala.collection.immutable.VectorBuilder.$plus$eq(Vector.scala:706)
                                                  //|   at scala.collection.immutable.VectorBuilder.$plus$eq(Vector.scala:692)
                                                  //|   at scala.collection.generic.Growable$$anonfun$$plus$plus$eq$1.apply(Grow
                                                  //| able.scala:48)
                                                  //|   at scala.collection.generic.Growable$$anonfun$$plus$plus$eq$1.apply(Grow
                                                  //| able.scala:48)
                                                  //|   at scala.collection.immutable.Range.foreach(Range.scala:141)
                                                  //|   at scala.collection.generic.Growable$class.$plus$plus$eq(Growable.scala:
                                                  //| 48)
                                                  //|   at scala.collection.immutable.VectorBuilder.$plus$plus$eq(Vector.scala:7
                                                  //| 16)
                                                  //|   at scala.collection.immutable.VectorBuilder.$plus$plus$eq(Vector.scala:6
                                                  //| 92)
                                                  //|   at scala.collection.TraversableLike$class.to(TraversableLike.scala:629)
                                                  //|   at scala.collect
                                                  //| Output exceeds cutoff limit.
}

Here it is again, this time with -Xmx4g:

object HeapKiller {
 (1 to 1e8.toInt).toVector.length                 //> res0: Int = 100000000
}

This change also brings to light a followup issue: the worksheet plugin doesn't seem to notify the user if the JVM failed to launch.

Imagine I changed the VM arguments to "-Xmx1024" (a common mistake, even though "1k ought to be enough for anybody" -me). Upon saving the worksheet and triggering an evaluation, I would see the the worksheet just sit and do nothing. Even the Eclipse error log remains empty.

Peeking into the underlying output stream, I was able to see the expected error:

Error occurred during initialization of VM
Incompatible minimum and maximum heap sizes specified

I assume there's also a non-zero exit status to latch on to.

At any rate, the pull request will be sent within the hour.

[dotta]

Much better, but you will notice how the text fields have become quite narrow. This makes it difficult for editing or perusal of VM arguments.

Other than introducing code for custom layout on the preferences page (beyond the scope of this change), the easiest thing to do could be to trim the cutoff preference description to something shorter like "Output character limit per statement". However, I've left things as they are in the latter screenshot for now.

I very much agree. I'm in favour to change this to "Output character limit per statement".

This change also brings to light a followup issue: the worksheet plugin doesn't seem to notify the user if the JVM failed to launch.

Imagine I changed the VM arguments to "-Xmx1024" (a common mistake, even though "1k ought to be enough for anybody" -me). Upon saving the worksheet and triggering an evaluation, I would see the the worksheet just sit and do nothing. Even the Eclipse error log remains empty.

Peeking into the underlying output stream, I was able to see the expected error:

Error occurred during initialization of VM Incompatible minimum and maximum heap sizes specified

I assume there's also a non-zero exit status to latch on to.

I agree this is an issue. I commented about it directly in the PR. If possible, the best way I see to tackle this is validating the user input. If that's not feasible, we should make sure to report an error to the user when VM can't be correctly initialized. I think this should be in the form of an error dialog, i.e., it has to be very clear for the user why the worksheet isn't working. WDYT?

Thanks for the great work!

[nadavwr]

I'll change the field label as suggested before I update the pull request tomorrow.

Regarding validation:

I wasn't able to find such a library, so I played around with JVM argument validation for a bit and found it interesting -- turns out 80% of such validation can be accomplished by simply parsing java -XX:PrintFlagsFinal -version to build a symbol table, then using that to parse user input. I've started a small stand alone library using Scala's parser combinators, and will have it up on GitHub soon.

I think validating VM arguments will be a killer IDE feature, but only if uniformly available (it isn't available in Eclipse AFAIK).

On the other hand That's also an argument against the feature: why would you add JVM argument validation for only this single feature, where that feature isn't available anywhere else? IMO, If you could do that for Scala IDE launch configurations in general, then you'd be talking :-) Then again, you'd be improving the Eclipse user experience in a way unrelated to Scala, so I still don't know. I'll be happy to contribute the library and integrate validation if you think you can use it.

Regardless of validation, and more importantly as well, I think the user must always be made aware of any failure to launch the JVM. Since the error report is available in the process output stream, that could either be accomplished by piping it into the worksheet, popping an error dialog, or both. A nice combination could be a dialog 'Error launching JVM: see worksheet for details', with the full output available there.

A final thought regarding worksheet-specific JVM arguments: we could do something like VIM modelines, but for JVM arguments.

[dragos]

It would be cool to have such a library, but I agree with you it's much more important to report all errors to the user, especially when the JVM failed to launch.

I guess we could use the library to validate launch configurations, and maybe it's a cool project in itself.

[nadavwr]

Just pushed jvmargs-validator (couldn't think of a better name). Would be glad to contribute it.

Sneak peek:

val cmdLine = "-XX:YakShaving=1000 -XX:MaxHeapSize=infinite"
ArgsValidator.validate("java", cmdLine) foreach println

will print the ParseResults:

[1.5] failure: unknown parameter
-XX:YakShaving=1000
    ^
[1.17] failure: 32-bit unsigned integer expected
-XX:MaxHeapSize=infinite
                ^

It's a bit of a long shot (I have some free time...), but I've been thinking about this within the context of worksheets: Consider something along the lines of VIM modelines. The first/last 5 lines of a file will be searched for directives, including JVM arguments for that specific worksheet.

It could use some @elidable comment-like function to wrap a string, to be used along with a validating string interpolator I can provide. e.g.:

object MySheet {
  comment(jvmarg"-Xmx1g") // <-- will compile, will cause worksheet to run with -Xmx1g
  // or instead:
  comment(jvmarg"-XmxThousand") // <-- will not compile, will receive a nice error message

  // where comment() is some elidable function I could have swore already
  // existed, but I can't find.

  println("Hello, world")
}

Anyway, I'll be back on track with error reporting tomorrow...

[nadavwr]

Default-sized preferences page now looks like so: I suppose this is as good as can be without resorting to custom layouts.

Providing a -Xmx100m argument as above works out nicely: Where did the extra 5m go? shrug

Providing a bogus main class (e.g., some.main.Class) results in a message to standard error:

Whereas providing a maximum heap size that is too small (or smaller than -Xms for that matter), results in a message to standard output:

Providing arguments such as -version result in... silence. The process will terminate with exit value 0, so no error dialog will appear. In order to address those cases, perhaps streaming output to document header or footer can be considered.

Explicitly specifying sys.exit(1) will cause whatever output is currently available in stdoutWriter or the stream to appear in the message. Without having examined that in depth, I can report that this seems to be the output from the last few commands.

Further work needed (IMO): In retrospect (having just added commit comments), I think the way I handled flushing the output stream and assembling the termination message could have been more graceful.

Further work needed II: Build kitty seems to fail on worksheet-2.1-2.9-pr-validator. 2.9..., and I used string interpolation. Fix coming right up.

[dragos]

@nadavwr The jvm validation library looks really good. The only problem I see is cross-compilation: we're building on top of different Scala versions, so all our dependencies have to be built against all Scala versions we're supporting (for instance, 2.11.0 milestones). It's an additional burden for library owners. If you are motivated to maintain and publish it for 2.9/2.10/2.11 (we'll probably drop 2.9 very soon), it's all good :). Otherwise it's "nice to have", but not vital.