[ https://issues.apache.org/jira/browse/FLINK-5066?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15719898#comment-15719898 ]
ASF GitHub Bot commented on FLINK-5066: --------------------------------------- Github user uce commented on the issue: https://github.com/apache/flink/pull/2806 Very good changes in general. The following should be addressed before merging though: - Add tests to `EventSerializerTest` with all currently available events (including `OTHER_EVENT` instances). - As a special case it's important to make sure that the ByteBuffer position indexes are not affected by doing a `isEvent` check, because a following deserialization will fail otherwise. The following example fails because of this: ```java ByteBuffer serializedEvent = EventSerializer.toSerializedEvent(EndOfPartitionEvent.INSTANCE); assertTrue(EventSerializer.isEvent(serializedEvent, EndOfPartitionEvent.class, cl)); EndOfPartitionEvent event = (EndOfPartitionEvent) EventSerializer.fromSerializedEvent(serializedEvent, cl); ``` For `Buffer` instances it works, because `Buffer#getNioBuffer()` always wraps the memory in a new `ByteBuffer` instance. For the tests, this can be covered by serializing an event, doing the `isEvent` check on the serialized buffer and then trying to deserialize the same serialized event buffer (like above). - The check is not used in `LocalInputChannel` any more, but only in `PartitionRequestQueue`. Let's move it there. - In general the EventSerializer interface could be made a little less overloaded by only exposing one variant (Buffer/ByteBuffer). That's indepedent of this issue though. > prevent LocalInputChannel#getNextBuffer from de-serialising all events when > looking for EndOfPartitionEvent only > ---------------------------------------------------------------------------------------------------------------- > > Key: FLINK-5066 > URL: https://issues.apache.org/jira/browse/FLINK-5066 > Project: Flink > Issue Type: Improvement > Components: Network > Reporter: Nico Kruber > Assignee: Nico Kruber > > LocalInputChannel#getNextBuffer de-serialises all incoming events on the > lookout for an EndOfPartitionEvent. > Instead, if EventSerializer offered a function to check for an event type > only without de-serialising the whole event, we could save some resources. -- This message was sent by Atlassian JIRA (v6.3.4#6332)