{"action":"created","issue":{"url":"https://api.github.com/repos/apache/spark/issues/5606","labels_url":"https://api.github.com/repos/apache/spark/issues/5606/labels{/name}","comments_url":"https://api.github.com/repos/apache/spark/issues/5606/comments","events_url":"https://api.github.com/repos/apache/spark/issues/5606/events","html_url":"https://github.com/apache/spark/pull/5606","id":69773187,"number":5606,"title":"[SPARK-3386] Share and reuse SerializerInstances in shuffle paths","user":{"login":"JoshRosen","id":50748,"avatar_url":"https://avatars.githubusercontent.com/u/50748?v=3","gravatar_id":"","url":"https://api.github.com/users/JoshRosen","html_url":"https://github.com/JoshRosen","followers_url":"https://api.github.com/users/JoshRosen/followers","following_url":"https://api.github.com/users/JoshRosen/following{/other_user}","gists_url":"https://api.github.com/users/JoshRosen/gists{/gist_id}","starred_url":"https://api.github.com/users/JoshRosen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JoshRosen/subscriptions","organizations_url":"https://api.github.com/users/JoshRosen/orgs","repos_url":"https://api.github.com/users/JoshRosen/repos","events_url":"https://api.github.com/users/JoshRosen/events{/privacy}","received_events_url":"https://api.github.com/users/JoshRosen/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":18,"created_at":"2015-04-21T07:53:39Z","updated_at":"2015-05-20T20:00:03Z","closed_at":"2015-04-21T23:25:12Z","pull_request":{"url":"https://api.github.com/repos/apache/spark/pulls/5606","html_url":"https://github.com/apache/spark/pull/5606","diff_url":"https://github.com/apache/spark/pull/5606.diff","patch_url":"https://github.com/apache/spark/pull/5606.patch"},"body":"This patch modifies several shuffle-related code paths to share and re-use SerializerInstances instead of creating new ones.  Some serializers, such as KryoSerializer or SqlSerializer, can be fairly expensive to create or may consume moderate amounts of memory, so it's probably best to avoid unnecessary serializer creation in hot code paths.\r\n\r\nThe key change in this patch is modifying `getDiskWriter()` / `DiskBlockObjectWriter` to accept `SerializerInstance`s instead of `Serializer`s (which are factories for instances).  This allows the disk writer's creator to decide whether the serializer instance can be shared or re-used.\r\n\r\nThe rest of the patch modifies several write and read paths to use shared serializers.  One big win is in `ShuffleBlockFetcherIterator`, where we used to create a new serializer per received block.  Similarly, the shuffle write path used to create a new serializer per file even though in many cases only a single thread would be writing to a file at a time.\r\n\r\nI made a small serializer reuse optimization in CoarseGrainedExecutorBackend as well, since it seemed like a small and obvious improvement."},"comment":{"url":"https://api.github.com/repos/apache/spark/issues/comments/104017534","html_url":"https://github.com/apache/spark/pull/5606#issuecomment-104017534","issue_url":"https://api.github.com/repos/apache/spark/issues/5606","id":104017534,"user":{"login":"JoshRosen","id":50748,"avatar_url":"https://avatars.githubusercontent.com/u/50748?v=3","gravatar_id":"","url":"https://api.github.com/users/JoshRosen","html_url":"https://github.com/JoshRosen","followers_url":"https://api.github.com/users/JoshRosen/followers","following_url":"https://api.github.com/users/JoshRosen/following{/other_user}","gists_url":"https://api.github.com/users/JoshRosen/gists{/gist_id}","starred_url":"https://api.github.com/users/JoshRosen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JoshRosen/subscriptions","organizations_url":"https://api.github.com/users/JoshRosen/orgs","repos_url":"https://api.github.com/users/JoshRosen/repos","events_url":"https://api.github.com/users/JoshRosen/events{/privacy}","received_events_url":"https://api.github.com/users/JoshRosen/received_events","type":"User","site_admin":false},"created_at":"2015-05-20T20:00:03Z","updated_at":"2015-05-20T20:00:03Z","body":"Hi @carrino,\r\n\r\nThanks for spotting this issue.  I've managed to write a regression test which exposes this bug: https://github.com/JoshRosen/spark/commit/71845e3d7805741dd830cdd66d8a49c24f2bdc95\r\n\r\nThis is only a problem when reference tracking is enabled _and_ auto-reset has been disabled by a user's custom Kryo registrator.  I think that this issue should be fairly easy to fix by adding `reset` calls at the end of `serialize` and `serializeStream`.  I'll file a JIRA for this and put together a pull request shortly, then ping you for review."}}