You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by twilmes <gi...@git.apache.org> on 2016/05/07 03:34:50 UTC
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1295 Precompile Script...
GitHub user twilmes opened a pull request:
https://github.com/apache/incubator-tinkerpop/pull/307
TINKERPOP-1295 Precompile ScriptInputFormat scripts once during initialization of ScriptRecordReader
This update precompiles an input script and then reads the input file using the compiled script instead of repeatedly calling the engine.eval(). This should cut down on the time spent repeatedly eval-ing the input script.
I ran a quick and dirty benchmark on my measly macbook with SparkGraphComputer, 2 workers. `g.V().count()` on a test file with 250,000 vertices and a simple `.groovy` script to read it in.
Average of 10 runs
-------------------------
before (TP 3.2.0 - engine.eval): 14975.7 ms
after (TP-1295 - w/ compiled script): 10163.6 ms
`mvn clean install` success
VOTE: +1
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP-1295
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-tinkerpop/pull/307.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #307
----
commit be6f8d79f606cc0a39dd45457e26328dd31e5636
Author: Ted Wilmes <tw...@gmail.com>
Date: 2016-05-06T19:54:54Z
Precompile scripts during ScriptRecordReader initialization.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1295 Precompile Script...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-tinkerpop/pull/307
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1295 Precompile Script...
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/307#issuecomment-217633168
Perfect. This will come in handy with all my Friendster benchmarking that uses `ScriptInputFormat`.
Did a code review, makes perfect sense.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1295 Precompile Script...
Posted by spmallette <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1295 Precompile Script...
Posted by PommeVerte <gi...@git.apache.org>.
Github user PommeVerte commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/307#issuecomment-218174710
`mvn clean install` runs for me as well and the changes make sense. :)
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---