Github user spmallette commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/307#issuecomment-218128585
This is an interesting change. I was surprised to see that much difference
between `compile()` and `eval()`. `eval()` ultimately calls compile but does
cache the compile script for future use. I would have thought that would have
narrowed the gap you managed to get between the two calls. There's a fair bit
of code in `eval()` that gets executed with or without the cache as compared to
`compile` so perhaps that has something to do with it. As the execution here
is about usage of the same script over and over there is little need for
`eval()` so this change makes sense to me.
I will only note that this forces the `ScriptEngine` we use to for
`ScriptRecordReader` to implement `Compileable`. Not all implementations do
that and it is not a required interface to the best of my recollection. That
probably doesn't matter too much as we've currently hardcoded the groovy script
engine in here.
Ran `mvn clean install` to success.
VOTE +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
