You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/04/22 19:19:19 UTC
[GitHub] [druid] gianm opened a new pull request, #12475: For the various Yielder objects, don't create new Yielders and instead mutate state.
gianm opened a new pull request, #12475:
URL: https://github.com/apache/druid/pull/12475
@cheddar wrote this patch, so I can't take credit for it. But I did test it, and it seems to work as advertised. It's part of a series of work we were doing to reduce allocations in the query path, which also includes #12468 and #12474.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on a diff in pull request #12475: For the various Yielder objects, don't create new Yielders and instead mutate state.
Posted by GitBox <gi...@apache.org>.
clintropolis commented on code in PR #12475:
URL: https://github.com/apache/druid/pull/12475#discussion_r856511211
##########
core/src/main/java/org/apache/druid/common/guava/CombiningSequence.java:
##########
@@ -94,52 +110,37 @@ public <OutType> Yielder<OutType> toYielder(OutType initValue, final YieldingAcc
private <OutType> Yielder<OutType> makeYielder(
final Yielder<T> yielder,
- final CombiningYieldingAccumulator<OutType, T> combiningAccumulator,
- boolean finalValue
+ final CombiningYieldingAccumulator<OutType, T> combiningAccumulator
)
{
- final Yielder<T> finalYielder;
- final OutType retVal;
- final boolean finalFinalValue;
-
- if (!yielder.isDone()) {
- retVal = combiningAccumulator.getRetVal();
- finalYielder = null;
- finalFinalValue = false;
- } else {
- if (!finalValue && combiningAccumulator.accumulatedSomething()) {
- combiningAccumulator.accumulateLastValue();
- retVal = combiningAccumulator.getRetVal();
- finalFinalValue = true;
-
- if (!combiningAccumulator.yielded()) {
- return Yielders.done(retVal, yielder);
- } else {
- finalYielder = Yielders.done(null, yielder);
- }
- } else {
- return Yielders.done(combiningAccumulator.getRetVal(), yielder);
- }
- }
-
-
return new Yielder<OutType>()
{
+ private Yielder<T> myYielder = yielder;
+ private CombiningYieldingAccumulator<OutType, T> accum = combiningAccumulator;
Review Comment:
nit: this can be final
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekagarwal87 closed pull request #12475: For the various Yielder objects, don't create new Yielders and instead mutate state.
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #12475: For the various Yielder objects, don't create new Yielders and instead mutate state.
URL: https://github.com/apache/druid/pull/12475
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm merged pull request #12475: For the various Yielder objects, don't create new Yielders and instead mutate state.
Posted by GitBox <gi...@apache.org>.
gianm merged PR #12475:
URL: https://github.com/apache/druid/pull/12475
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org