gianm commented on PR #15552:
URL: https://github.com/apache/druid/pull/15552#issuecomment-1881749096
>"makeItReallyImmutable" approach
> it could achieved with a call to `expr.getCachedStringValue()` ; which is
not nice...probably adding a `makeImmutable()` or something...
Yeah I was thinking of something like `.immutable()` or `.threadSafe()` on
`ExprEval`.
> the `singleThreaded` appraoch
> * it seems to me it adds one more extra conditional to be
evaluated...but no big deal I think the branch predictor can do a pretty good
job with it..
> * wouldn't the above sample still retain the issue in `ExprEval` ?
and it would still need the `makeItReallyImmutable` as well in some form?
Ah, I forgot to add `this.expr = expr;` in the second `DoubleExpr`
constructor. I just edited my comment to add that in. I had imagined that
`expr` would only be set on `DoubleExpr` that are returned by
`singleThreaded()`. The usage would be like:
```
// shared across threads
Expr expr = Parser.parse(expression, macroTable);
// in each processing thread
Expr localExpr = expr.singleThreaded();
ObjectBindings bindings = // build bindings somehow
while (... data ...) {
// do stuff with localExpr.eval(bindings);
}
```
> instead of caching I wonder why not remove this conditional caching layer
-
> if say on a DoubleExpr the asString() method will be called most of the
time then why have a DoubleExpr with caching ? why not a ConstantStringExpr ?
That's a good question… it's definitely better to do this if we have the
info about what type we're going to want from each Expr. Certainly sometimes we
do. I'm not sure if we always have it. I think it's related to `ExpressionPlan`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
