Thans, Eugene. The change doesn't seem wrong and it looks like default behavior remains intact (i.e. we continue to use a UUID if no id is supplied). We save a bit of processing with this change as we avoid a UUID generation for no reason. I assume that this change is more about "correctness" than a performance enhancement as I always had it in my mind that even under extreme contention UUID generation was still insanely cheap ....is that correct?
On Tue, Dec 5, 2017 at 10:12 PM, Robert Dale <[email protected]> wrote: > I think this makes sense. Create an issue with your description then > create the PR on tp32 and reference that issue. > > Robert Dale > > On Tue, Dec 5, 2017 at 8:19 PM, Eugene Chung <[email protected]> > wrote: > > > Hello, TinkerPop developers! > > > > > > I’m the user of JanusGraph, as you may know, which is the graph database > > on top of the gremlin server. > > Recently on the investigation of why the gremlin-server uses UUID as its > > request id, > > I saw that the Builder class of org.apache.tinkerpop.gremlin. > driver.message.RequestMessage > > class sets its requestId field as UUID.randomUUID() by default. > > > > But I think it should be fixed not to be set by default. The reasons are > > below; > > > > - UUID.randomUUID() uses SecureRandom which grabs the lock at JVM level, > > which means whole threads calling this API compete against each > other. > > - Getting random value from SecureRandom is somewhat CPU-intensive job. > > - If a gremlin client sends requestId by itself, the costs above are > > useless. > > > > > > If you guys think my suggestion is reasonable, I will make this as git > > pull request. > > > > > > Regards, > > Eugene > > > > > > > > p.s. Here’s my diff. > > > > Index: gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/ > > driver/message/RequestMessage.java > > IDEA additional info: > > Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP > > <+>UTF-8 > > =================================================================== > > --- gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/ > > driver/message/RequestMessage.java (revision > > fa0a0ee64331bb1e67248137bd97fe001554ac10) > > +++ gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/ > > driver/message/RequestMessage.java (date 1512465656000) > > @@ -112,7 +112,7 @@ > > */ > > public static final class Builder { > > public static final String OP_PROCESSOR_NAME = ""; > > - private UUID requestId = UUID.randomUUID(); > > + private UUID requestId; > > private String op; > > private String processor = OP_PROCESSOR_NAME; > > private Map<String, Object> args = new HashMap<>(); > > @@ -155,7 +155,7 @@ > > * Create the request message given the settings provided to the > > {@link Builder}. > > */ > > public RequestMessage create() { > > - return new RequestMessage(requestId, op, processor, args); > > + return new RequestMessage(requestId == null ? > > UUID.randomUUID() : requestId, op, processor, args); > > } > > } > > > > > > >
