[GitHub] johnyangk commented on a change in pull request #178: [NEMO-317] Optimize triggering logic in GroupByKeyAndWindowDoFnTransform

[GitBox]

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:
us...@infra.apache.org


With regards,
Apache Git Services