{"action":"created","issue":{"url":"https://api.github.com/repos/apache/flink/issues/1517","repository_url":"https://api.github.com/repos/apache/flink","labels_url":"https://api.github.com/repos/apache/flink/issues/1517/labels{/name}","comments_url":"https://api.github.com/repos/apache/flink/issues/1517/comments","events_url":"https://api.github.com/repos/apache/flink/issues/1517/events","html_url":"https://github.com/apache/flink/pull/1517","id":127267451,"number":1517,"title":"[FLINK-3477] [runtime] Add hash-based combine strategy for ReduceFunction.","user":{"login":"ggevay","id":11377925,"avatar_url":"https://avatars.githubusercontent.com/u/11377925?v=3","gravatar_id":"","url":"https://api.github.com/users/ggevay","html_url":"https://github.com/ggevay","followers_url":"https://api.github.com/users/ggevay/followers","following_url":"https://api.github.com/users/ggevay/following{/other_user}","gists_url":"https://api.github.com/users/ggevay/gists{/gist_id}","starred_url":"https://api.github.com/users/ggevay/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ggevay/subscriptions","organizations_url":"https://api.github.com/users/ggevay/orgs","repos_url":"https://api.github.com/users/ggevay/repos","events_url":"https://api.github.com/users/ggevay/events{/privacy}","received_events_url":"https://api.github.com/users/ggevay/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":48,"created_at":"2016-01-18T16:53:48Z","updated_at":"2016-06-18T12:50:59Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/apache/flink/pulls/1517","html_url":"https://github.com/apache/flink/pull/1517","diff_url":"https://github.com/apache/flink/pull/1517.diff","patch_url":"https://github.com/apache/flink/pull/1517.patch"},"body":"This adds ReduceHashTable, and modifies ReduceCombineDriver to use it instead of sorting. When an input element comes in, the driver probes the table for the key, and either inserts the element if it is not present, or updates it with one reduce step. When memory runs out, all the current elements (partial aggregates) in the table are sent to the output.\r\n\r\nI haven't yet deleted the code of the sort based solution from the driver, but it seems that the hash-based solution is always faster (sometimes by a factor of 3), so there isn't a need for dynamically choosing between them. I will do some more performance tests to confirm this, and then probably delete the sort based code. The hash-based approach also has the advantage that the memory used is proportional to the number of different keys, instead of the number of input elements.\r\n\r\nThe interface of ReduceHashTable is a subset of the interface of CompactingHashTable, so in theory, it could replace CompactingHashTable everywhere (this is only JoinWithSolutionSet and coGroupWithSolutionSet, if I'm not mistaken). I haven't actually tried this yet though, but if you think that it worth to give it a go, then I will try it, and run some real performance tests. I've already added ReduceHashTable to HashTablePerformanceComparison, and when there is plenty of memory available, ReduceHashTable is about 5-10% faster than CompactingHashTable. It also uses ~15% less memory, and it also has the advantage that if the sizes of the records don't change during the updates, then no compactings are needed (because records are updated in-place), so performance doesn't degrade when we are close to occupying all the memory. (In contrast to CompactingHashTable, where updates start getting slow, when there is little free memory left.)\r\n\r\nFor details about the operation of ReduceHashTable, see the comment at the beginning of the file.\r\n\r\nI've changed the ctor of RandomAccessInputView to not do a seek. (This behaviour is more practical for me, because I want to create an instance before allocating memory segments to it.) I verified at most places of usage that the code does an explicit seek after the ctor, but I couldn't see this in AbstractBlockResettableIterator, so I inserted an explicit seek there.\r\n\r\nI fixed a small bug in HashTablePerformanceComparison: the *-1 was not doing anything, because that value was always 0 there.\r\n\r\nThe predefined aggregations (eg. DataSet.sum()) currently use groupReduce. I'm not sure what was the reason for this originally, but in light of this speedup of reduce now, maybe this could be reconsidered. (And the same stands for DataSet.distinct().)\r\n\r\nAdditionally, I have plans for making a different hash table for handling the case when the serializer can tell the size of the records. This would allow open addressing to be used with linear probing, which would avoid a few cache misses per element access, so I'm expecting maybe a 2x speedup from this. (This open addressing hash table would actually be simpler than ReduceHashTable.) The difficulty with this is that currently the serializers are not very smart in this respect (I mean knowing the size), so some improvements would be needed for them, but I have some ideas for this."},"comment":{"url":"https://api.github.com/repos/apache/flink/issues/comments/226940863","html_url":"https://github.com/apache/flink/pull/1517#issuecomment-226940863","issue_url":"https://api.github.com/repos/apache/flink/issues/1517","id":226940863,"user":{"login":"ggevay","id":11377925,"avatar_url":"https://avatars.githubusercontent.com/u/11377925?v=3","gravatar_id":"","url":"https://api.github.com/users/ggevay","html_url":"https://github.com/ggevay","followers_url":"https://api.github.com/users/ggevay/followers","following_url":"https://api.github.com/users/ggevay/following{/other_user}","gists_url":"https://api.github.com/users/ggevay/gists{/gist_id}","starred_url":"https://api.github.com/users/ggevay/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ggevay/subscriptions","organizations_url":"https://api.github.com/users/ggevay/orgs","repos_url":"https://api.github.com/users/ggevay/repos","events_url":"https://api.github.com/users/ggevay/events{/privacy}","received_events_url":"https://api.github.com/users/ggevay/received_events","type":"User","site_admin":false},"created_at":"2016-06-18T12:50:59Z","updated_at":"2016-06-18T12:50:59Z","body":"I have rebased to the current master."}}
{"actor":{"display_login":"ggevay"}}