{"action":"closed","number":2,"pull_request":{"url":"https://api.github.com/repos/Groostav/XStream-GG/pulls/2","id":34333031,"html_url":"https://github.com/Groostav/XStream-GG/pull/2","diff_url":"https://github.com/Groostav/XStream-GG/pull/2.diff","patch_url":"https://github.com/Groostav/XStream-GG/pull/2.patch","issue_url":"https://api.github.com/repos/Groostav/XStream-GG/issues/2","number":2,"state":"closed","locked":false,"title":"added Mapper methods and granularity for marshalling","user":{"login":"Groostav","id":1680611,"avatar_url":"https://avatars.githubusercontent.com/u/1680611?v=3","gravatar_id":"","url":"https://api.github.com/users/Groostav","html_url":"https://github.com/Groostav","followers_url":"https://api.github.com/users/Groostav/followers","following_url":"https://api.github.com/users/Groostav/following{/other_user}","gists_url":"https://api.github.com/users/Groostav/gists{/gist_id}","starred_url":"https://api.github.com/users/Groostav/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Groostav/subscriptions","organizations_url":"https://api.github.com/users/Groostav/orgs","repos_url":"https://api.github.com/users/Groostav/repos","events_url":"https://api.github.com/users/Groostav/events{/privacy}","received_events_url":"https://api.github.com/users/Groostav/received_events","type":"User","site_admin":false},"body":"Simplified my previous implementation such that it doesn't use enums and doesn't modify any public facing API's.\r\n\r\nTo properly preserve backwards compatibility, my new method must delegate to the extra-arg free one, such that the chain of wrappers, on a call to the new method, also includes calling the old method. Since the method is looking for any positive result (ie, once anybody returns true in any context we're done), the solution to this was simply to have\r\n\r\n```Java\r\npublic boolean isImmutableType(Class typeName, boolean includeBackwardsCompatible){\r\n    return isImmutableType(typeName) || wrapped.isImmutableType(typeName, includeBackwardsCompatible)\r\n}\r\n```\r\n\r\nThis has the effect of invoking any user-written backwards-compatible-agnostic code to look for a result, and then look through the defaults in the proper scope.\r\n\r\nThe one issue I have with this implementation is that the performance of the `Mapper.isImmutableType(Class, boolean)` is porportional to the _square_ of the number of wrappers.\r\n\r\nFor example, if we have:\r\n- StringeyMapper extends MapperWrapper \r\n- DomainishMapper extends MapperWrapper\r\n- ImmutableMapper extends MapperWrapper\r\n\r\nand we get them wrapped in the sequence\r\n\r\n`StringeyMapper` delegates to `DomainishMapper` delegates to `ImmutableMapper` delegates to `defaultMapper`\r\n\r\nthen the call stack would be\r\n\r\ncaller | call-site\r\n--- | ---\r\n_consumer_ | `stringyMapper.isImmutableType(t, true)`\r\nstringyMapper | `stringyMapper.isImmutableType(t)`\r\nstringyMapper | `domainishMapper.isImmutableType(t)`\r\ndomainishMapper | `immutableMapper.isImmutableType(t)`\r\nimmutableMapper | `defaultMapper.isImmutableType(t)`\r\nimmutableMapper | 1-arg method on defaultMapper returns\r\ndomainishMapper | 1-arg method on immutableMapper returns\r\nstringyMapper | 1-arg method on domainishMapper returns\r\nstringyMapper | `domainishMapper.isImmutableType(t, true)`\r\ndomainishMapper | **redundantly** `domainishMapper.isImmutableType(t)`\r\ndomainishMapper | **redundantly** `immutableMapper.isImmutableType(t)`\r\nimmutableMapper | **redundantly** `defaultMapper.isImmutableType(t)`\r\nimmutableMapper | 1-arg method on defaultMapper returns\r\ndomainishMapper | 1-arg method on immutableMapper returns\r\ndomainishMapper | `immutableMapper.isImmutableType(t, true)`\r\nimmutableMapper | **redundantly** `immutableMapper.isImmutableType(t)`\r\nimmutableMapper | **redundantly** `defaultMapper.isImmutableType(t)`\r\nimmutableMapper | 1-arg method on defaultMapper returns\r\nimmutableMapper | defaultMapper.isImmutableType(t, true);\r\nimmutableMapper | 2-arg method on defaultMapper returns\r\ndomainishMapper | 2-arg method on immutableMapper returns\r\nstringeyMapper | 2-arg method on domainishMapper returns\r\n_consumer_ | 2-arg method on stringeyMapper returns \r\n\r\nIf I'm not mistaken this should be equal to numWrappers ^2  /2, but 13 != 8 so I might be mistaken.\r\n\r\nThis means that any user-written code for Types must be re-entrant, which was not a requirement before (though no sane implementation wouldn't be). Further it means a user that puts a break-point inside his `isIMmutableType` overload is going to see it hit _alot_ more frequently, though that doesn't seem like too much of a concern to me.\r\n\r\nA bigger concern to me was performance: all those extra calls might make a negative impact on performance. To test this I created an array list of 1,000,000 ints, and implemented a solution that kept a map of currently-inspected types which it referenced, so as to remove the redundant calls. This implementation was slightly slower than the implementation that had the redundant calls, so performance appears not to be an issue, unless I made an error in writing the performance test.\r\n\r\nusing the test\r\n```Java\r\n        ArrayList<Integer> ints = new ArrayList<>();\r\n        for(int i = 0; i < 1_000_000; i++){\r\n            ints.add(i);\r\n        }\r\n\r\n        StopWatch watch = new StopWatch();\r\n        watch.start();\r\n        String document = xstream.toXML(ints);\r\n        System.out.println(\"took \" + watch.getTime() + \"ms to serialize\");\r\n        watch.stop();\r\n        watch.reset();\r\n        watch.start();\r\n\r\n        Object result = xstream.fromXML(document);\r\n        System.out.print(\"took \" + watch.getTime() + \"ms to deserialize\");\r\n```\r\n\r\nand the following to address the problem\r\n```Java\r\n//as a static field --ThreadLocal's have been around since java 1.2, and anonymous classes since 1.1\r\nprivate static final ThreadLocal<Set<Class>> inspectingTypes = new ThreadLocal<Set<Class>>(){\r\n    protected Set<Class> initialValue() { return new HashSet<>(); }\r\n};\r\n\r\n//in the isImmutableType(Class, boolean) method\r\nSet<Class> classCallStack = inspectingTypes.get();\r\n\r\nif( ! classCallStack.contains(type)) try {\r\n    classCallStack.add(type);\r\n\r\n    return isImmutableValueType(type)\r\n            || wrapped.isImmutableValueType(type, includeBackwardsCompatibleTypes);\r\n}\r\nfinally{\r\n    classCallStack.remove(type);\r\n}\r\nelse{\r\n    return wrapped.isImmutableValueType(type, includeBackwardsCompatibleTypes);\r\n}\r\n```\r\n\r\nI got\r\n\r\nscenario | serialization time | deserialization time\r\n--- | --- | ---\r\nwith the problem | 931ms | 1720ms\r\nwithout the problem | 928ms | 1672ms\r\nwith the wierd stack-set fix | 948ms | 1722ms\r\n\r\nwhich to me means its all a wash, thus probably just accepting these extra calls is OK.\r\n","created_at":"2015-04-29T04:35:55Z","updated_at":"2015-04-29T04:49:35Z","closed_at":"2015-04-29T04:49:35Z","merged_at":null,"merge_commit_sha":"14a17fe8a962a13b89b158f82b143495a62cefb4","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/Groostav/XStream-GG/pulls/2/commits","review_comments_url":"https://api.github.com/repos/Groostav/XStream-GG/pulls/2/comments","review_comment_url":"https://api.github.com/repos/Groostav/XStream-GG/pulls/comments{/number}","comments_url":"https://api.github.com/repos/Groostav/XStream-GG/issues/2/comments","statuses_url":"https://api.github.com/repos/Groostav/XStream-GG/statuses/7188b52411b11f642b0840e9c8c49a662dacd54e","head":{"label":"Groostav:XSTR_773_exclude_immutable_types_from_xpath_map_on_unmarshal","ref":"XSTR_773_exclude_immutable_types_from_xpath_map_on_unmarshal","sha":"7188b52411b11f642b0840e9c8c49a662dacd54e","user":{"login":"Groostav","id":1680611,"avatar_url":"https://avatars.githubusercontent.com/u/1680611?v=3","gravatar_id":"","url":"https://api.github.com/users/Groostav","html_url":"https://github.com/Groostav","followers_url":"https://api.github.com/users/Groostav/followers","following_url":"https://api.github.com/users/Groostav/following{/other_user}","gists_url":"https://api.github.com/users/Groostav/gists{/gist_id}","starred_url":"https://api.github.com/users/Groostav/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Groostav/subscriptions","organizations_url":"https://api.github.com/users/Groostav/orgs","repos_url":"https://api.github.com/users/Groostav/repos","events_url":"https://api.github.com/users/Groostav/events{/privacy}","received_events_url":"https://api.github.com/users/Groostav/received_events","type":"User","site_admin":false},"repo":{"id":34405582,"name":"XStream-GG","full_name":"Groostav/XStream-GG","owner":{"login":"Groostav","id":1680611,"avatar_url":"https://avatars.githubusercontent.com/u/1680611?v=3","gravatar_id":"","url":"https://api.github.com/users/Groostav","html_url":"https://github.com/Groostav","followers_url":"https://api.github.com/users/Groostav/followers","following_url":"https://api.github.com/users/Groostav/following{/other_user}","gists_url":"https://api.github.com/users/Groostav/gists{/gist_id}","starred_url":"https://api.github.com/users/Groostav/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Groostav/subscriptions","organizations_url":"https://api.github.com/users/Groostav/orgs","repos_url":"https://api.github.com/users/Groostav/repos","events_url":"https://api.github.com/users/Groostav/events{/privacy}","received_events_url":"https://api.github.com/users/Groostav/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/Groostav/XStream-GG","description":"fork of XStream for xpath unmarshalling fix (XSTR-773)","fork":false,"url":"https://api.github.com/repos/Groostav/XStream-GG","forks_url":"https://api.github.com/repos/Groostav/XStream-GG/forks","keys_url":"https://api.github.com/repos/Groostav/XStream-GG/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Groostav/XStream-GG/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Groostav/XStream-GG/teams","hooks_url":"https://api.github.com/repos/Groostav/XStream-GG/hooks","issue_events_url":"https://api.github.com/repos/Groostav/XStream-GG/issues/events{/number}","events_url":"https://api.github.com/repos/Groostav/XStream-GG/events","assignees_url":"https://api.github.com/repos/Groostav/XStream-GG/assignees{/user}","branches_url":"https://api.github.com/repos/Groostav/XStream-GG/branches{/branch}","tags_url":"https://api.github.com/repos/Groostav/XStream-GG/tags","blobs_url":"https://api.github.com/repos/Groostav/XStream-GG/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Groostav/XStream-GG/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Groostav/XStream-GG/git/refs{/sha}","trees_url":"https://api.github.com/repos/Groostav/XStream-GG/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Groostav/XStream-GG/statuses/{sha}","languages_url":"https://api.github.com/repos/Groostav/XStream-GG/languages","stargazers_url":"https://api.github.com/repos/Groostav/XStream-GG/stargazers","contributors_url":"https://api.github.com/repos/Groostav/XStream-GG/contributors","subscribers_url":"https://api.github.com/repos/Groostav/XStream-GG/subscribers","subscription_url":"https://api.github.com/repos/Groostav/XStream-GG/subscription","commits_url":"https://api.github.com/repos/Groostav/XStream-GG/commits{/sha}","git_commits_url":"https://api.github.com/repos/Groostav/XStream-GG/git/commits{/sha}","comments_url":"https://api.github.com/repos/Groostav/XStream-GG/comments{/number}","issue_comment_url":"https://api.github.com/repos/Groostav/XStream-GG/issues/comments{/number}","contents_url":"https://api.github.com/repos/Groostav/XStream-GG/contents/{+path}","compare_url":"https://api.github.com/repos/Groostav/XStream-GG/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Groostav/XStream-GG/merges","archive_url":"https://api.github.com/repos/Groostav/XStream-GG/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Groostav/XStream-GG/downloads","issues_url":"https://api.github.com/repos/Groostav/XStream-GG/issues{/number}","pulls_url":"https://api.github.com/repos/Groostav/XStream-GG/pulls{/number}","milestones_url":"https://api.github.com/repos/Groostav/XStream-GG/milestones{/number}","notifications_url":"https://api.github.com/repos/Groostav/XStream-GG/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Groostav/XStream-GG/labels{/name}","releases_url":"https://api.github.com/repos/Groostav/XStream-GG/releases{/id}","created_at":"2015-04-22T17:37:36Z","updated_at":"2015-04-29T03:36:52Z","pushed_at":"2015-04-29T04:35:55Z","git_url":"git://github.com/Groostav/XStream-GG.git","ssh_url":"git@github.com:Groostav/XStream-GG.git","clone_url":"https://github.com/Groostav/XStream-GG.git","svn_url":"https://github.com/Groostav/XStream-GG","homepage":null,"size":0,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"Groostav:master","ref":"master","sha":"2aadc5950ab0a058c9af2c24326d50a2f0004e18","user":{"login":"Groostav","id":1680611,"avatar_url":"https://avatars.githubusercontent.com/u/1680611?v=3","gravatar_id":"","url":"https://api.github.com/users/Groostav","html_url":"https://github.com/Groostav","followers_url":"https://api.github.com/users/Groostav/followers","following_url":"https://api.github.com/users/Groostav/following{/other_user}","gists_url":"https://api.github.com/users/Groostav/gists{/gist_id}","starred_url":"https://api.github.com/users/Groostav/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Groostav/subscriptions","organizations_url":"https://api.github.com/users/Groostav/orgs","repos_url":"https://api.github.com/users/Groostav/repos","events_url":"https://api.github.com/users/Groostav/events{/privacy}","received_events_url":"https://api.github.com/users/Groostav/received_events","type":"User","site_admin":false},"repo":{"id":34405582,"name":"XStream-GG","full_name":"Groostav/XStream-GG","owner":{"login":"Groostav","id":1680611,"avatar_url":"https://avatars.githubusercontent.com/u/1680611?v=3","gravatar_id":"","url":"https://api.github.com/users/Groostav","html_url":"https://github.com/Groostav","followers_url":"https://api.github.com/users/Groostav/followers","following_url":"https://api.github.com/users/Groostav/following{/other_user}","gists_url":"https://api.github.com/users/Groostav/gists{/gist_id}","starred_url":"https://api.github.com/users/Groostav/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Groostav/subscriptions","organizations_url":"https://api.github.com/users/Groostav/orgs","repos_url":"https://api.github.com/users/Groostav/repos","events_url":"https://api.github.com/users/Groostav/events{/privacy}","received_events_url":"https://api.github.com/users/Groostav/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/Groostav/XStream-GG","description":"fork of XStream for xpath unmarshalling fix (XSTR-773)","fork":false,"url":"https://api.github.com/repos/Groostav/XStream-GG","forks_url":"https://api.github.com/repos/Groostav/XStream-GG/forks","keys_url":"https://api.github.com/repos/Groostav/XStream-GG/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Groostav/XStream-GG/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Groostav/XStream-GG/teams","hooks_url":"https://api.github.com/repos/Groostav/XStream-GG/hooks","issue_events_url":"https://api.github.com/repos/Groostav/XStream-GG/issues/events{/number}","events_url":"https://api.github.com/repos/Groostav/XStream-GG/events","assignees_url":"https://api.github.com/repos/Groostav/XStream-GG/assignees{/user}","branches_url":"https://api.github.com/repos/Groostav/XStream-GG/branches{/branch}","tags_url":"https://api.github.com/repos/Groostav/XStream-GG/tags","blobs_url":"https://api.github.com/repos/Groostav/XStream-GG/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Groostav/XStream-GG/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Groostav/XStream-GG/git/refs{/sha}","trees_url":"https://api.github.com/repos/Groostav/XStream-GG/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Groostav/XStream-GG/statuses/{sha}","languages_url":"https://api.github.com/repos/Groostav/XStream-GG/languages","stargazers_url":"https://api.github.com/repos/Groostav/XStream-GG/stargazers","contributors_url":"https://api.github.com/repos/Groostav/XStream-GG/contributors","subscribers_url":"https://api.github.com/repos/Groostav/XStream-GG/subscribers","subscription_url":"https://api.github.com/repos/Groostav/XStream-GG/subscription","commits_url":"https://api.github.com/repos/Groostav/XStream-GG/commits{/sha}","git_commits_url":"https://api.github.com/repos/Groostav/XStream-GG/git/commits{/sha}","comments_url":"https://api.github.com/repos/Groostav/XStream-GG/comments{/number}","issue_comment_url":"https://api.github.com/repos/Groostav/XStream-GG/issues/comments{/number}","contents_url":"https://api.github.com/repos/Groostav/XStream-GG/contents/{+path}","compare_url":"https://api.github.com/repos/Groostav/XStream-GG/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Groostav/XStream-GG/merges","archive_url":"https://api.github.com/repos/Groostav/XStream-GG/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Groostav/XStream-GG/downloads","issues_url":"https://api.github.com/repos/Groostav/XStream-GG/issues{/number}","pulls_url":"https://api.github.com/repos/Groostav/XStream-GG/pulls{/number}","milestones_url":"https://api.github.com/repos/Groostav/XStream-GG/milestones{/number}","notifications_url":"https://api.github.com/repos/Groostav/XStream-GG/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Groostav/XStream-GG/labels{/name}","releases_url":"https://api.github.com/repos/Groostav/XStream-GG/releases{/id}","created_at":"2015-04-22T17:37:36Z","updated_at":"2015-04-29T03:36:52Z","pushed_at":"2015-04-29T04:35:55Z","git_url":"git://github.com/Groostav/XStream-GG.git","ssh_url":"git@github.com:Groostav/XStream-GG.git","clone_url":"https://github.com/Groostav/XStream-GG.git","svn_url":"https://github.com/Groostav/XStream-GG","homepage":null,"size":0,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/Groostav/XStream-GG/pulls/2"},"html":{"href":"https://github.com/Groostav/XStream-GG/pull/2"},"issue":{"href":"https://api.github.com/repos/Groostav/XStream-GG/issues/2"},"comments":{"href":"https://api.github.com/repos/Groostav/XStream-GG/issues/2/comments"},"review_comments":{"href":"https://api.github.com/repos/Groostav/XStream-GG/pulls/2/comments"},"review_comment":{"href":"https://api.github.com/repos/Groostav/XStream-GG/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/Groostav/XStream-GG/pulls/2/commits"},"statuses":{"href":"https://api.github.com/repos/Groostav/XStream-GG/statuses/7188b52411b11f642b0840e9c8c49a662dacd54e"}},"merged":false,"mergeable":true,"mergeable_state":"clean","merged_by":null,"comments":2,"review_comments":2,"commits":1,"additions":325,"deletions":193,"changed_files":13}}