{"action":"created","issue":{"url":"https://api.github.com/repos/tensorflow/tensorflow/issues/6528","repository_url":"https://api.github.com/repos/tensorflow/tensorflow","labels_url":"https://api.github.com/repos/tensorflow/tensorflow/issues/6528/labels{/name}","comments_url":"https://api.github.com/repos/tensorflow/tensorflow/issues/6528/comments","events_url":"https://api.github.com/repos/tensorflow/tensorflow/issues/6528/events","html_url":"https://github.com/tensorflow/tensorflow/pull/6528","id":197757100,"number":6528,"title":"Support reference counting on Java Tensor class","user":{"login":"EronWright","id":1775518,"avatar_url":"https://avatars.githubusercontent.com/u/1775518?v=3","gravatar_id":"","url":"https://api.github.com/users/EronWright","html_url":"https://github.com/EronWright","followers_url":"https://api.github.com/users/EronWright/followers","following_url":"https://api.github.com/users/EronWright/following{/other_user}","gists_url":"https://api.github.com/users/EronWright/gists{/gist_id}","starred_url":"https://api.github.com/users/EronWright/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/EronWright/subscriptions","organizations_url":"https://api.github.com/users/EronWright/orgs","repos_url":"https://api.github.com/users/EronWright/repos","events_url":"https://api.github.com/users/EronWright/events{/privacy}","received_events_url":"https://api.github.com/users/EronWright/received_events","type":"User","site_admin":false},"labels":[{"id":404586594,"url":"https://api.github.com/repos/tensorflow/tensorflow/labels/awaiting%20googler","name":"awaiting googler","color":"b60205","default":false},{"id":300136587,"url":"https://api.github.com/repos/tensorflow/tensorflow/labels/cla:%20yes","name":"cla: yes","color":"009800","default":false}],"state":"open","locked":false,"assignee":{"login":"asimshankar","id":16018,"avatar_url":"https://avatars.githubusercontent.com/u/16018?v=3","gravatar_id":"","url":"https://api.github.com/users/asimshankar","html_url":"https://github.com/asimshankar","followers_url":"https://api.github.com/users/asimshankar/followers","following_url":"https://api.github.com/users/asimshankar/following{/other_user}","gists_url":"https://api.github.com/users/asimshankar/gists{/gist_id}","starred_url":"https://api.github.com/users/asimshankar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/asimshankar/subscriptions","organizations_url":"https://api.github.com/users/asimshankar/orgs","repos_url":"https://api.github.com/users/asimshankar/repos","events_url":"https://api.github.com/users/asimshankar/events{/privacy}","received_events_url":"https://api.github.com/users/asimshankar/received_events","type":"User","site_admin":false},"assignees":[{"login":"asimshankar","id":16018,"avatar_url":"https://avatars.githubusercontent.com/u/16018?v=3","gravatar_id":"","url":"https://api.github.com/users/asimshankar","html_url":"https://github.com/asimshankar","followers_url":"https://api.github.com/users/asimshankar/followers","following_url":"https://api.github.com/users/asimshankar/following{/other_user}","gists_url":"https://api.github.com/users/asimshankar/gists{/gist_id}","starred_url":"https://api.github.com/users/asimshankar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/asimshankar/subscriptions","organizations_url":"https://api.github.com/users/asimshankar/orgs","repos_url":"https://api.github.com/users/asimshankar/repos","events_url":"https://api.github.com/users/asimshankar/events{/privacy}","received_events_url":"https://api.github.com/users/asimshankar/received_events","type":"User","site_admin":false}],"milestone":null,"comments":2,"created_at":"2016-12-27T20:59:12Z","updated_at":"2016-12-29T07:53:16Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/tensorflow/tensorflow/pulls/6528","html_url":"https://github.com/tensorflow/tensorflow/pull/6528","diff_url":"https://github.com/tensorflow/tensorflow/pull/6528.diff","patch_url":"https://github.com/tensorflow/tensorflow/pull/6528.patch"},"body":"Closes #6524 \r\n\r\n- introduces RefCounted and AbstractRefCounted, with tests\r\n- implements reference counting on Tensor (which no longer implements\r\nAutoCloseable)"},"comment":{"url":"https://api.github.com/repos/tensorflow/tensorflow/issues/comments/269594209","html_url":"https://github.com/tensorflow/tensorflow/pull/6528#issuecomment-269594209","issue_url":"https://api.github.com/repos/tensorflow/tensorflow/issues/6528","id":269594209,"user":{"login":"asimshankar","id":16018,"avatar_url":"https://avatars.githubusercontent.com/u/16018?v=3","gravatar_id":"","url":"https://api.github.com/users/asimshankar","html_url":"https://github.com/asimshankar","followers_url":"https://api.github.com/users/asimshankar/followers","following_url":"https://api.github.com/users/asimshankar/following{/other_user}","gists_url":"https://api.github.com/users/asimshankar/gists{/gist_id}","starred_url":"https://api.github.com/users/asimshankar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/asimshankar/subscriptions","organizations_url":"https://api.github.com/users/asimshankar/orgs","repos_url":"https://api.github.com/users/asimshankar/repos","events_url":"https://api.github.com/users/asimshankar/events{/privacy}","received_events_url":"https://api.github.com/users/asimshankar/received_events","type":"User","site_admin":false},"created_at":"2016-12-29T07:53:16Z","updated_at":"2016-12-29T07:53:16Z","body":"Thanks for the suggestion and pull request @EronWright .\r\n\r\nHowever, I'm a bit skeptical on this one. I did consider this but felt that the language support for `AutoCloseable` makes it a more appropriate choice at this lower level. Some reservations:\r\n\r\n- I fear that it will be much easier for developers to forget to `unref()` than forget to close an `AutoCloseable`, mostly because of language support (and implied un-idiomatic nature of reference counting in Java).\r\n- There is asymmetry between creation and cleanup that is very subtle. For example, if `Session.Runner.run()` isn't called after `Session.Runner.feed()` (perhaps because of some exception thrown in intervening code), then the `Tensor`s are not cleaned up. I see that the PR also uses finalizers, but relying on that is generally unadvisable given that there are no guarantees on garbage collection and that `Tensor` objects can have large memory footprints\r\n\r\nA wrapper for reference counting and/or a list of `AutoCloseable`s seems easy enough, but perhaps I'm not fully appreciating the complexity of that. I had thought of something like:\r\n\r\n```java\r\npublic class AutoCloseableList implements List<AutoCloseable>, AutoCloseable {\r\n   // ...\r\n   public void close() {\r\n      for (AutoCloseable c : list) {\r\n         c.close();\r\n      }\r\n   }\r\n}\r\n```\r\n\r\nto make it easier to work with `Session.Runner`, or with reference counting something like:\r\n\r\n```java\r\npublic class RefCounted<E implements AutoCloseable> {\r\n   public E ref() { ... }\r\n   public boolean unref() {\r\n      if (refcnt-- == 0) {\r\n        object.close();\r\n        return true;\r\n     }\r\n    return false;\r\n  }\r\n}\r\n```\r\n\r\nWhich I was hoping would be trivial and easy to incorporate adapters into Netty-like settings (and I'd be happy to add those to `org.tensorflow.util`)\r\n\r\nCould you perhaps elaborate on why you think such wrappers would be burdensome? Is there some example code you can share that can maybe guide us in finding the right balance? For example, I feel `LabelImage.java` is easier to read without the reference counting. But I am wide open to looking at more use cases.\r\n\r\nCC @jhseu "}}
{"actor":{"display_login":"asimshankar"}}