-
Notifications
You must be signed in to change notification settings - Fork 22
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
Version the graph examples #85
Conversation
* Run the Ruby examples on GraalVM distributions that support it. * Added GraalVM 22.3.1 to the list of GraalVM distributions used for graph generation. * Restructure the way we run commands to enable code reuse.
…erence example graphs.
…rmat. GraalVM used to ship as a JVM into which one could install multiple languages with a tool named `gu`. The `gu` utility was removed and languages are now avaliable as Maven artifacts. While that's considerably more convenient for applications embedding Truffle languages, it complicates using each language's launcher. Consequently, we now need to install separate distributions for each language in order to run the examples for that language.
…he reference examples to use GraalVM 23.1.2.
tools/generate-examples.rb
Outdated
require "pathname" | ||
|
||
VERBOSE = false | ||
EXAMPLES_DIR = File.expand_path("#{__dir__}/../examples", __dir__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd write this as
EXAMPLES_DIR = File.expand_path("#{__dir__}/../examples", __dir__) | |
EXAMPLES_DIR = File.expand_path("../examples", __dir__) |
As is, I think the second arg will be unused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. I didn't even realize there was a second argument.
def run_command(graalvm, args, options = {}) | ||
if VERBOSE | ||
log(graalvm, " Command: #{args.join(" ")}") | ||
end | ||
|
||
IO.popen(args, options) | ||
end | ||
|
||
def run_java(graalvm, bin, dump_level, *args) | ||
FileUtils.rm_rf("./graal_dumps") | ||
|
||
pipe = run_command(graalvm, [ | ||
"#{bin}/java", \ | ||
"-XX:+PrintCompilation", | ||
"-Dgraal.PrintGraphWithSchedule=true", | ||
"-Dgraal.Dump=:#{dump_level}", | ||
] + args) | ||
|
||
yield pipe | ||
|
||
sleep(3) | ||
Process.kill("KILL", pipe.pid) | ||
Process.wait(pipe.pid) | ||
end | ||
|
||
def run_js(graalvm, bin, *args) | ||
FileUtils.rm_rf("graal_dumps") | ||
|
||
pipe = run_command( | ||
graalvm, | ||
[ | ||
"#{bin}/node", | ||
"--experimental-options", | ||
"--engine.CompileOnly=fib", | ||
"--engine.TraceCompilation", | ||
"--engine.MultiTier=false", | ||
graalvm.options_version == 1 ? "--engine.Inlining=false" : "--compiler.Inlining=false", | ||
"--vm.Dgraal.Dump=Truffle:1", | ||
] + args, | ||
{ err: [:child, :out] }, | ||
) | ||
|
||
yield pipe | ||
|
||
sleep(3) | ||
Process.kill("KILL", pipe.pid) | ||
Process.wait(pipe.pid) | ||
end | ||
|
||
def run_ruby(graalvm, bin, *args) | ||
FileUtils.rm_rf("./graal_dumps") | ||
|
||
pipe = run_command( | ||
graalvm, | ||
[ | ||
"#{bin}/ruby", | ||
"--jvm", | ||
"--experimental-options", | ||
"--engine.TraceCompilation", | ||
graalvm.options_version == 1 ? "--engine.NodeSourcePositions" : "--compiler.NodeSourcePositions", | ||
"--engine.MultiTier=false", | ||
"--vm.Dgraal.Dump=Truffle:1", | ||
] + args, | ||
{ err: [:child, :out] }, | ||
) | ||
|
||
yield pipe | ||
|
||
sleep(3) | ||
Process.kill("KILL", pipe.pid) | ||
Process.wait(pipe.pid) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's quite a bit more functionality duplicated across the methods that you could move to this function.
def run_command(graalvm, args, options = {}) | |
if VERBOSE | |
log(graalvm, " Command: #{args.join(" ")}") | |
end | |
IO.popen(args, options) | |
end | |
def run_java(graalvm, bin, dump_level, *args) | |
FileUtils.rm_rf("./graal_dumps") | |
pipe = run_command(graalvm, [ | |
"#{bin}/java", \ | |
"-XX:+PrintCompilation", | |
"-Dgraal.PrintGraphWithSchedule=true", | |
"-Dgraal.Dump=:#{dump_level}", | |
] + args) | |
yield pipe | |
sleep(3) | |
Process.kill("KILL", pipe.pid) | |
Process.wait(pipe.pid) | |
end | |
def run_js(graalvm, bin, *args) | |
FileUtils.rm_rf("graal_dumps") | |
pipe = run_command( | |
graalvm, | |
[ | |
"#{bin}/node", | |
"--experimental-options", | |
"--engine.CompileOnly=fib", | |
"--engine.TraceCompilation", | |
"--engine.MultiTier=false", | |
graalvm.options_version == 1 ? "--engine.Inlining=false" : "--compiler.Inlining=false", | |
"--vm.Dgraal.Dump=Truffle:1", | |
] + args, | |
{ err: [:child, :out] }, | |
) | |
yield pipe | |
sleep(3) | |
Process.kill("KILL", pipe.pid) | |
Process.wait(pipe.pid) | |
end | |
def run_ruby(graalvm, bin, *args) | |
FileUtils.rm_rf("./graal_dumps") | |
pipe = run_command( | |
graalvm, | |
[ | |
"#{bin}/ruby", | |
"--jvm", | |
"--experimental-options", | |
"--engine.TraceCompilation", | |
graalvm.options_version == 1 ? "--engine.NodeSourcePositions" : "--compiler.NodeSourcePositions", | |
"--engine.MultiTier=false", | |
"--vm.Dgraal.Dump=Truffle:1", | |
] + args, | |
{ err: [:child, :out] }, | |
) | |
yield pipe | |
sleep(3) | |
Process.kill("KILL", pipe.pid) | |
Process.wait(pipe.pid) | |
end | |
def run_command(graalvm, args, options = {}) | |
if VERBOSE | |
log(graalvm, " Command: #{args.join(" ")}") | |
end | |
FileUtils.rm_rf("./graal_dumps") | |
pipe = IO.popen(args, options) | |
yield pipe | |
sleep(3) | |
Process.kill("KILL", pipe.pid) | |
Process.wait(pipe.pid) | |
end | |
def run_java(graalvm, bin, dump_level, *args) | |
run_command(graalvm, [ | |
"#{bin}/java", \ | |
"-XX:+PrintCompilation", | |
"-Dgraal.PrintGraphWithSchedule=true", | |
"-Dgraal.Dump=:#{dump_level}", | |
] + args) | |
end | |
def run_js(graalvm, bin, *args) | |
run_command( | |
graalvm, | |
[ | |
"#{bin}/node", | |
"--experimental-options", | |
"--engine.CompileOnly=fib", | |
"--engine.TraceCompilation", | |
"--engine.MultiTier=false", | |
graalvm.options_version == 1 ? "--engine.Inlining=false" : "--compiler.Inlining=false", | |
"--vm.Dgraal.Dump=Truffle:1", | |
] + args, | |
{ err: [:child, :out] }, | |
) | |
end | |
def run_ruby(graalvm, bin, *args) | |
run_command( | |
graalvm, | |
[ | |
"#{bin}/ruby", | |
"--jvm", | |
"--experimental-options", | |
"--engine.TraceCompilation", | |
graalvm.options_version == 1 ? "--engine.NodeSourcePositions" : "--compiler.NodeSourcePositions", | |
"--engine.MultiTier=false", | |
"--vm.Dgraal.Dump=Truffle:1", | |
] + args, | |
{ err: [:child, :out] }, | |
) | |
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also noticed that { err: [:child, :out] },
is only used for js and ruby (not java).
We're currently discarding stdout so I think the only difference is that the java command's stderr would be printed to console.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with the duplication. All I wanted was a way to log the command easily. I'm already a bit uneasy with the multiple levels required to execute a command. I think the simplicity of the original bash-like implementation was a lot easier to follow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of notes but looks fine.
* Added optional logging of commands being run. * Fixed deprecated usage of Graal options. * Cleaned up the reference to the examples directory.
d5cd1c9
to
738a957
Compare
This is a fixed branch for #84. I had merged that PR already before realizing that I lost the Ruby graphs for GraalVM 21.2.0. Since those cannot be regenerated without a patched Graal, we want to retain the originals. I reverted the PR on main and reworked the branch. This PR supplants #84 (see original for more context).