{"action":"created","issue":{"url":"https://api.github.com/repos/java-native-access/jna/issues/954","repository_url":"https://api.github.com/repos/java-native-access/jna","labels_url":"https://api.github.com/repos/java-native-access/jna/issues/954/labels{/name}","comments_url":"https://api.github.com/repos/java-native-access/jna/issues/954/comments","events_url":"https://api.github.com/repos/java-native-access/jna/issues/954/events","html_url":"https://github.com/java-native-access/jna/pull/954","id":318666983,"node_id":"MDExOlB1bGxSZXF1ZXN0MTg0ODA0NDE4","number":954,"title":"define the structure's field order with metadata","user":{"login":"idosu","id":10791145,"node_id":"MDQ6VXNlcjEwNzkxMTQ1","avatar_url":"https://avatars1.githubusercontent.com/u/10791145?v=4","gravatar_id":"","url":"https://api.github.com/users/idosu","html_url":"https://github.com/idosu","followers_url":"https://api.github.com/users/idosu/followers","following_url":"https://api.github.com/users/idosu/following{/other_user}","gists_url":"https://api.github.com/users/idosu/gists{/gist_id}","starred_url":"https://api.github.com/users/idosu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/idosu/subscriptions","organizations_url":"https://api.github.com/users/idosu/orgs","repos_url":"https://api.github.com/users/idosu/repos","events_url":"https://api.github.com/users/idosu/events{/privacy}","received_events_url":"https://api.github.com/users/idosu/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":15,"created_at":"2018-04-28T20:37:31Z","updated_at":"2018-06-17T15:28:13Z","closed_at":"2018-05-10T12:17:15Z","author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/java-native-access/jna/pulls/954","html_url":"https://github.com/java-native-access/jna/pull/954","diff_url":"https://github.com/java-native-access/jna/pull/954.diff","patch_url":"https://github.com/java-native-access/jna/pull/954.patch"},"body":"define a `Structure` using an annotation instead of implementing a method.\r\n instead of this:\r\n``` java\r\nclass Parent extends Structure {\r\n    public int n;\r\n    public String s;\r\n    protected List<String> getFieldOrder() {\r\n        return Arrays.asList(\"n\", \"s\");\r\n    }\r\n}\r\nclass Son extends Parent {\r\n    public double d;\r\n    public char c;\r\n    protected List<String> getFieldOrder() {\r\n        List<String> fields = new LinkedList<String>(super.getFieldOrder());\r\n        fields.addAll(Arrays.asList(\"d\", \"c\"));\r\n        return fields;\r\n    }\r\n}\r\n```\r\n\r\nuse this:\r\n``` java\r\n\r\n@FieldOrder({ \"n\", \"s\" })\r\nclass Parent extends Structure {\r\n    public int n;\r\n    public String s;\r\n}\r\n@FieldOrder({ \"d\", \"c\" })\r\nclass Son extends Parent {\r\n    public double d;\r\n    public char c;\r\n}\r\n```\r\n\r\nIn both examples calling `new Son().getFieldOrder()` will resolve in this list `[ \"a\", \"b\", \"c\", \"d\" ]`\r\n\r\nIt improves readability and makes that the user could not forget to add super's fields because it is added automatically by the library"},"comment":{"url":"https://api.github.com/repos/java-native-access/jna/issues/comments/397886412","html_url":"https://github.com/java-native-access/jna/pull/954#issuecomment-397886412","issue_url":"https://api.github.com/repos/java-native-access/jna/issues/954","id":397886412,"node_id":"MDEyOklzc3VlQ29tbWVudDM5Nzg4NjQxMg==","user":{"login":"twall","id":756909,"node_id":"MDQ6VXNlcjc1NjkwOQ==","avatar_url":"https://avatars0.githubusercontent.com/u/756909?v=4","gravatar_id":"","url":"https://api.github.com/users/twall","html_url":"https://github.com/twall","followers_url":"https://api.github.com/users/twall/followers","following_url":"https://api.github.com/users/twall/following{/other_user}","gists_url":"https://api.github.com/users/twall/gists{/gist_id}","starred_url":"https://api.github.com/users/twall/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/twall/subscriptions","organizations_url":"https://api.github.com/users/twall/orgs","repos_url":"https://api.github.com/users/twall/repos","events_url":"https://api.github.com/users/twall/events{/privacy}","received_events_url":"https://api.github.com/users/twall/received_events","type":"User","site_admin":false},"created_at":"2018-06-17T15:28:13Z","updated_at":"2018-06-17T15:28:13Z","author_association":"CONTRIBUTOR","body":"Nuno,\n\n  Just curious, is the few hundred fat structs scenario actually moving\ndata to and from native in all those structs?  The first goal in JNA's\ndefault behavior is to make things easy, and there are usually less-easy\napproaches to improve performance.\n\n  Re: ArrayList, that's what the original field management used.   To me,\nit seems a simpler implementation for the field management, since the lists\nare always append-only.\n\n  Matthias is right about usage of LinkedList, it likely is due to one or\nmore contributions from the same author; if the net benefit of a patch is\njudged to be positive, then the reviewers aren't too nitpicky.  Sometimes\nusing ArrayList vs LinkedList is just due to one's personal habits.\n\nOn Sat, Jun 16, 2018 at 8:05 PM, Nuno Cruces <notifications@github.com>\nwrote:\n\n> Adding some context.\n>\n> I just had to drop JNA (for JNI) in a project because of how slow JNA was\n> at marshaling an array of a few hundred fat structs. Something like 90% of\n> the time was spent marshaling and GCing garbage created for marshaling.\n> This isn't a chatty API (it's a single call, which does some actual work),\n> but there's a lot of marshaling involved (both in and out).\n>\n> Just creating a \"contiguous\" array of structs (with Struct.toArray) to\n> pass as an argument was painfully slow. I came up with a workaround, but\n> had to make my structs Cloneable because most of the overhead is from the\n> constructor.\n>\n> The change in this PR, *as it is*, makes things measurably worse, because\n> iterating a LinkedList with List.get is obviously slow (I've profiled it,\n> but can't really make an MCVE out of it).\n>\n> I thought it was a simple, non contentious, improvement. You use an\n> ArrayList here\n> <https://github.com/java-native-access/jna/blob/7ac44fee3d6b0e47de6d5d10c32be258b9ed1bef/src/com/sun/jna/Structure.java#L993>,\n> after all.\n>\n> I also had other ideas, but I'd need time to mature them, and I've moved\n> on for now.\n> ------------------------------\n>\n> But, I see you guys tend to use LinkedList in a *lot* of places.\n>\n> Another example:\n> https://github.com/java-native-access/jna/blob/\n> 7b66282949dc17739cfac02baf291f03378c4783/src/com/sun/jna/\n> CallbackReference.java#L399\n>\n> The person who wrote this might be surprised to know that, the first thing\n> the LinkedList constructor in OpenJDK is going to do is call\n> Collection.toArray (see here\n> <http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/java/util/LinkedList.java#l408>),\n> so you might as well do that instead.\n>\n> If my benchmarks are not enough to convince you that, for smallish lists\n> (N<100), ArrayList consumes less memory, creates less garbage, iterates\n> faster, even handles the worse case List.add(0, e) scenario faster, then\n> I'm afraid nothing will.\n>\n> —\n> You are receiving this because you are subscribed to this thread.\n> Reply to this email directly, view it on GitHub\n> <https://github.com/java-native-access/jna/pull/954#issuecomment-397845720>,\n> or mute the thread\n> <https://github.com/notifications/unsubscribe-auth/AAuMrZxD1Fh54Y97cl-pY6nfaNa1J8Ndks5t9Z1hgaJpZM4TrjtB>\n> .\n>\n"}}
{"actor":{"display_login":"twall"}}