{"action":"created","issue":{"url":"https://api.github.com/repos/apache/spark/issues/21456","repository_url":"https://api.github.com/repos/apache/spark","labels_url":"https://api.github.com/repos/apache/spark/issues/21456/labels{/name}","comments_url":"https://api.github.com/repos/apache/spark/issues/21456/comments","events_url":"https://api.github.com/repos/apache/spark/issues/21456/events","html_url":"https://github.com/apache/spark/pull/21456","id":327471611,"node_id":"MDExOlB1bGxSZXF1ZXN0MTkxMjU4MTc0","number":21456,"title":"[SPARK-24356] [CORE] Duplicate strings in File.path managed by FileSegmentManagedBuffer","user":{"login":"countmdm","id":6394099,"node_id":"MDQ6VXNlcjYzOTQwOTk=","avatar_url":"https://avatars3.githubusercontent.com/u/6394099?v=4","gravatar_id":"","url":"https://api.github.com/users/countmdm","html_url":"https://github.com/countmdm","followers_url":"https://api.github.com/users/countmdm/followers","following_url":"https://api.github.com/users/countmdm/following{/other_user}","gists_url":"https://api.github.com/users/countmdm/gists{/gist_id}","starred_url":"https://api.github.com/users/countmdm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/countmdm/subscriptions","organizations_url":"https://api.github.com/users/countmdm/orgs","repos_url":"https://api.github.com/users/countmdm/repos","events_url":"https://api.github.com/users/countmdm/events{/privacy}","received_events_url":"https://api.github.com/users/countmdm/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":9,"created_at":"2018-05-29T20:16:55Z","updated_at":"2018-05-31T18:37:28Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/apache/spark/pulls/21456","html_url":"https://github.com/apache/spark/pull/21456","diff_url":"https://github.com/apache/spark/pull/21456.diff","patch_url":"https://github.com/apache/spark/pull/21456.patch"},"body":"This patch eliminates duplicate strings that come from the 'path' field of\r\njava.io.File objects created by FileSegmentManagedBuffer. That is, we want\r\nto avoid the situation when multiple File instances for the same pathname\r\n\"foo/bar\" are created, each with a separate copy of the \"foo/bar\" String\r\ninstance. In some scenarios such duplicate strings may waste a lot of memory\r\n(~ 10% of the heap). To avoid that, we intern the pathname with\r\nString.intern(), and before that we make sure that it's in a normalized\r\nform (contains no \"//\", \"///\" etc.) Otherwise, the code in java.io.File\r\nwould normalize it later, creating a new \"foo/bar\" String copy.\r\nUnfortunately, the normalization code that java.io.File uses internally\r\nis in the package-private class java.io.FileSystem, so we cannot call it\r\nhere directly.\r\n\r\n## What changes were proposed in this pull request?\r\n\r\nAdded code to ExternalShuffleBlockResolver.getFile(), that normalizes and then interns the pathname string before passing it to the File() constructor.\r\n\r\n## How was this patch tested?\r\n\r\nAdded unit test\r\n"},"comment":{"url":"https://api.github.com/repos/apache/spark/issues/comments/393633293","html_url":"https://github.com/apache/spark/pull/21456#issuecomment-393633293","issue_url":"https://api.github.com/repos/apache/spark/issues/21456","id":393633293,"node_id":"MDEyOklzc3VlQ29tbWVudDM5MzYzMzI5Mw==","user":{"login":"countmdm","id":6394099,"node_id":"MDQ6VXNlcjYzOTQwOTk=","avatar_url":"https://avatars3.githubusercontent.com/u/6394099?v=4","gravatar_id":"","url":"https://api.github.com/users/countmdm","html_url":"https://github.com/countmdm","followers_url":"https://api.github.com/users/countmdm/followers","following_url":"https://api.github.com/users/countmdm/following{/other_user}","gists_url":"https://api.github.com/users/countmdm/gists{/gist_id}","starred_url":"https://api.github.com/users/countmdm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/countmdm/subscriptions","organizations_url":"https://api.github.com/users/countmdm/orgs","repos_url":"https://api.github.com/users/countmdm/repos","events_url":"https://api.github.com/users/countmdm/events{/privacy}","received_events_url":"https://api.github.com/users/countmdm/received_events","type":"User","site_admin":false},"created_at":"2018-05-31T18:37:28Z","updated_at":"2018-05-31T18:37:28Z","author_association":"NONE","body":"If we don't do normalization ourselves, we may potentially run into the following: \r\n\r\npath = ...  // Produces \"foo//bar\"\r\npath = path.intern();  // Ok, no separate copies of \"foo//bar\" anymore\r\nFile f = new Fille(path);\r\n// Internally, code in File() constructor looks at the given path.\r\n// If path is not in the normalized form, it normalizes it, producing a new stirng \"foo/bar\"\r\nSystem.out.println(f.getPath());  // Prints \"foo/bar\"\r\n\r\nSince the code inside java.io.File doesn't do any string interning, we will keep canonicalizing the \"foo//bar\" string, but then java.io.File will still generate multiple copies of \"foo/bar\".\r\n"}}
{"actor":{"display_login":"countmdm"}}