{"action":"opened","issue":{"url":"https://api.github.com/repos/tdunning/t-digest/issues/53","labels_url":"https://api.github.com/repos/tdunning/t-digest/issues/53/labels{/name}","comments_url":"https://api.github.com/repos/tdunning/t-digest/issues/53/comments","events_url":"https://api.github.com/repos/tdunning/t-digest/issues/53/events","html_url":"https://github.com/tdunning/t-digest/issues/53","id":71396549,"number":53,"title":"smallByteSize methods are very trappy in many classes -- should be changed or have warnings in javadocs","user":{"login":"hossman","id":450139,"avatar_url":"https://avatars.githubusercontent.com/u/450139?v=3","gravatar_id":"","url":"https://api.github.com/users/hossman","html_url":"https://github.com/hossman","followers_url":"https://api.github.com/users/hossman/followers","following_url":"https://api.github.com/users/hossman/following{/other_user}","gists_url":"https://api.github.com/users/hossman/gists{/gist_id}","starred_url":"https://api.github.com/users/hossman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hossman/subscriptions","organizations_url":"https://api.github.com/users/hossman/orgs","repos_url":"https://api.github.com/users/hossman/repos","events_url":"https://api.github.com/users/hossman/events{/privacy}","received_events_url":"https://api.github.com/users/hossman/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2015-04-27T20:42:07Z","updated_at":"2015-04-27T20:42:07Z","closed_at":null,"body":"When integrating t-digest into Solr, the first patch contributed had code which looked like this...\r\n\r\n    ByteBuffer buf = ByteBuffer.allocate(tdigest.smallByteSize());\r\n    tdigest.asSmallBytes(buf);\r\n    res.add(\"percentiles\", buf.array());\r\n\r\n...the contributor who wrote that code did so based on the javadocs of `ArrayDigest.smallByteSize`, not realizing that under the covers hte internals of this method are:\r\n\r\n* allocate a ByteBuffer based on the result of `byteSize()`\r\n* do a complete serialization using `asSmallBytes(ByteBuffer)`\r\n* return the final position of the ByteBuffer\r\n\r\n...meaning that ultimately at least 2x the `smallByteSize` of RAM was being allocated everytime, and the ArrayDigest was being serialized twice.\r\n\r\nMethods like `ArrayDigest.smallByteSize` (`AVLTreeDigest.smallByteSize`, etc...) that have such a high internal cost should either be changed to just call `byteSize()` (since the overallocation the user will have to do is still better for space/time performance then doing that same overallocation internally plus a behind the scenes serialiation that is thrown away) or warn users about this heavy internal cost in the javadocs.\r\n"}}