carp84 commented on a change in pull request #8122: [FLINK-12121] [State Backends] Use composition instead of inheritance for the InternalKeyContext logic in backend URL: https://github.com/apache/flink/pull/8122#discussion_r273411703
########## File path: flink-runtime/src/main/java/org/apache/flink/runtime/state/AbstractKeyedStateBackend.java ########## @@ -201,37 +199,34 @@ public boolean deregisterKeySelectionListener(KeySelectionListener<K> listener) */ @Override public TypeSerializer<K> getKeySerializer() { - return keySerializerProvider.currentSchemaSerializer(); + return keyContext.getCurrentKeySerializer(); } /** * @see KeyedStateBackend */ @Override public K getCurrentKey() { - return currentKey; + return this.keyContext.getCurrentKey(); } /** * @see KeyedStateBackend */ - @Override public int getCurrentKeyGroupIndex() { - return currentKeyGroup; + return this.keyContext.getCurrentKeyGroupIndex(); } /** * @see KeyedStateBackend */ - @Override public int getNumberOfKeyGroups() { return numberOfKeyGroups; Review comment: Semantically calling `keyContext.getNumberOfKeyGroups` is equal to current implementation, but since `numberOfKeyGroups` is final, recording it in a local field could assure no method invocation happens during the `getNumberOfKeyGroups` call. I'm not sure whether the JIT compiler will optimize this, but I guess it's a Hotspot-dependent behavior, so for all final fields in `keyContext` I kept a local field in `AbstractKeyedStateBackend`. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services