johnyangk commented on a change in pull request #178: [NEMO-317] Optimize
triggering logic in GroupByKeyAndWindowDoFnTransform
URL: https://github.com/apache/incubator-nemo/pull/178#discussion_r241645692
##########
File path:
compiler/frontend/beam/src/main/java/org/apache/nemo/compiler/frontend/beam/transform/GroupByKeyAndWindowDoFnTransform.java
##########
@@ -282,69 +277,40 @@ private void triggerTimers(final K key,
return timerData;
}
- /**
- * State and timer internal.
- */
- final class StateAndTimerForKey {
- private StateInternals stateInternals;
- private TimerInternals timerInternals;
-
- /**
- * @param stateInternals state internals.
- * @param timerInternals timer internals.
- */
- StateAndTimerForKey(final StateInternals stateInternals,
- final TimerInternals timerInternals) {
- this.stateInternals = stateInternals;
- this.timerInternals = timerInternals;
- }
- }
/**
* InMemoryStateInternalsFactory.
*/
final class InMemoryStateInternalsFactory implements
StateInternalsFactory<K> {
- private final Map<K, StateAndTimerForKey> map;
+ private final Map<K, StateInternals> map;
- /**
- * @param map initial map.
- */
- InMemoryStateInternalsFactory(final Map<K, StateAndTimerForKey> map) {
- this.map = map;
+ InMemoryStateInternalsFactory() {
+ this.map = new HashMap<>();
}
@Override
public StateInternals stateInternalsForKey(final K key) {
- map.putIfAbsent(key, new
StateAndTimerForKey(InMemoryStateInternals.forKey(key), null));
- final StateAndTimerForKey stateAndTimerForKey = map.get(key);
- if (stateAndTimerForKey.stateInternals == null) {
- stateAndTimerForKey.stateInternals =
InMemoryStateInternals.forKey(key);
- }
- return stateAndTimerForKey.stateInternals;
+ map.computeIfAbsent(key, InMemoryStateInternals::forKey);
+ return map.get(key);
}
}
/**
- * InMemoryTimerInternalsFactory.
+ * InMemoryTimerInternalsFactory that hold all timer data of keys.
*/
final class InMemoryTimerInternalsFactory implements
TimerInternalsFactory<K> {
- private final Map<K, StateAndTimerForKey> map;
+ private final Map<K, NemoTimerInternals> timerInternalsMap = new
HashMap<>();
+ private ContextForTimer context;
Review comment:
I'd make this a field of `GroupByKeyAndWindowDoFnTransform`, as
`InMemoryTimerInternalsFactory.context` feels a little awkward.
Also add a comment:
// reused across different keys
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services