{"action":"created","comment":{"url":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/comments/191063133","pull_request_review_id":123570931,"id":191063133,"diff_hunk":"@@ -1342,9 +1342,9 @@ has to stop with an error. Duplicate keys are also prohibited by CBOR\n decoders that are using strict mode ({{strict-mode}}).\n \n The CBOR data model for maps does not allow ascribing semantics to the\n-order of the key/value pairs in the map representation.\n-Thus, it would be a very bad practice to define a CBOR-based protocol\n-in such a way that changing the key/value pair order in a map would\n+order of the key/value pairs in the map representation.  Thus, a\n+CBOR-based protocol MUST NOT specify that changing the key/value pair\n+order in a map would\n change the semantics, apart from trivial aspects (cache usage, etc.).","path":"draft-ietf-cbor-7049bis.md","position":null,"original_position":10,"commit_id":"545dbe14548b65c5e7e831b116f686150f58f6d1","original_commit_id":"8dff72f6986f5af009d67ceecd150b64b7138d0f","user":{"login":"BurtHarris","id":2715647,"avatar_url":"https://avatars0.githubusercontent.com/u/2715647?v=4","gravatar_id":"","url":"https://api.github.com/users/BurtHarris","html_url":"https://github.com/BurtHarris","followers_url":"https://api.github.com/users/BurtHarris/followers","following_url":"https://api.github.com/users/BurtHarris/following{/other_user}","gists_url":"https://api.github.com/users/BurtHarris/gists{/gist_id}","starred_url":"https://api.github.com/users/BurtHarris/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/BurtHarris/subscriptions","organizations_url":"https://api.github.com/users/BurtHarris/orgs","repos_url":"https://api.github.com/users/BurtHarris/repos","events_url":"https://api.github.com/users/BurtHarris/events{/privacy}","received_events_url":"https://api.github.com/users/BurtHarris/received_events","type":"User","site_admin":false},"body":"\r\nI think a formal declaration using RFC 2119 keywords is better than saying something vague like \"very bad practice\", but I'm not sure if this should be a MUST NOT or a SHOULD NOT.  Do you have specific reasoning?\r\n\r\nThe word 'trivial' is quite dismissive.   Using 'performance' or 'optimization' may be better but reordering MAY have significant performance impact on decoding CBOR for several reasons outside the control of the CBOR working group.\r\n\r\nConsider the case of a compiled language like C, which is decoding a CBOR map into a fixed struct.   Generating code to accept the keys in any order is certainly possible, but substantially more complicated than accepting them in some defined order, in both speed and code complexity.   On a highly memory constrained system that complexity and code size could be a concern.    \r\n\r\nAt the other end of the spectrum, for JavaScript on a server,  accepting the keys in any order seems to add almost nothing to code complexity, but if the order isn't consistent, it _can_ still have substantial runtime impact, including having previously JIT compiled code abandoned, leading to application code (as opposed to the serialization code) getting relegated to a interpreted implementation instead of using JIT optimizations.   \r\n\r\nWhile I agree in concept that ordering shouldn't matter, performance does matter, so it might be very reasonable for a CBOR-based protocol to specify a strongly preferred ordering, particularly when such ordering makes it into compiled machine code, either based on schema knowledge, hints, OR runtime optimizations like JIT.\r\n\r\nYou can say that if order matters, then a map isn't the best choice, and while I'd agree in general, embedding opinion into a MUST NOT requirement into an RFC should be carefully researched, so I'd be more comfortable with this being a SHOULD NOT in the next RFC.   \r\n\r\nI think its equally true that Generic Encoders and Decoders SHOULD NOT reorder keys without strong reason to do so.   Such strong reason might include schema knowledge or hints like tags, optimization, or even limitations in the runtime environment (e.g. Perl may not have a choice.)   But sophisticated runtimes, like the .NET runtime, have a HybridDictionary class which start out behaving in an key-order preserving way for small numbers of keys, but based on well researched information about the performance characteristics may switch to a hash-table on the fly.\r\n\r\n\r\n","created_at":"2018-05-27T02:49:11Z","updated_at":"2018-05-27T02:49:11Z","html_url":"https://github.com/cbor-wg/CBORbis/pull/20#discussion_r191063133","pull_request_url":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/20","author_association":"NONE","_links":{"self":{"href":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/comments/191063133"},"html":{"href":"https://github.com/cbor-wg/CBORbis/pull/20#discussion_r191063133"},"pull_request":{"href":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/20"}},"in_reply_to_id":184454445},"pull_request":{"url":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/20","id":177123139,"html_url":"https://github.com/cbor-wg/CBORbis/pull/20","diff_url":"https://github.com/cbor-wg/CBORbis/pull/20.diff","patch_url":"https://github.com/cbor-wg/CBORbis/pull/20.patch","issue_url":"https://api.github.com/repos/cbor-wg/CBORbis/issues/20","number":20,"state":"open","locked":false,"title":"Forbid CBOR-based protocols from depending on mapping order.","user":{"login":"jyasskin","id":83420,"avatar_url":"https://avatars1.githubusercontent.com/u/83420?v=4","gravatar_id":"","url":"https://api.github.com/users/jyasskin","html_url":"https://github.com/jyasskin","followers_url":"https://api.github.com/users/jyasskin/followers","following_url":"https://api.github.com/users/jyasskin/following{/other_user}","gists_url":"https://api.github.com/users/jyasskin/gists{/gist_id}","starred_url":"https://api.github.com/users/jyasskin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jyasskin/subscriptions","organizations_url":"https://api.github.com/users/jyasskin/orgs","repos_url":"https://api.github.com/users/jyasskin/repos","events_url":"https://api.github.com/users/jyasskin/events{/privacy}","received_events_url":"https://api.github.com/users/jyasskin/received_events","type":"User","site_admin":false},"body":"Except for being able to reject messages with a non-canonical order.","created_at":"2018-03-23T16:34:53Z","updated_at":"2018-05-27T02:49:11Z","closed_at":null,"merged_at":null,"merge_commit_sha":"c4b266d1c70470c6e3d3679bf044ea394819aebc","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/20/commits","review_comments_url":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/20/comments","review_comment_url":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/comments{/number}","comments_url":"https://api.github.com/repos/cbor-wg/CBORbis/issues/20/comments","statuses_url":"https://api.github.com/repos/cbor-wg/CBORbis/statuses/545dbe14548b65c5e7e831b116f686150f58f6d1","head":{"label":"jyasskin:mapping-order-in-protocols","ref":"mapping-order-in-protocols","sha":"545dbe14548b65c5e7e831b116f686150f58f6d1","user":{"login":"jyasskin","id":83420,"avatar_url":"https://avatars1.githubusercontent.com/u/83420?v=4","gravatar_id":"","url":"https://api.github.com/users/jyasskin","html_url":"https://github.com/jyasskin","followers_url":"https://api.github.com/users/jyasskin/followers","following_url":"https://api.github.com/users/jyasskin/following{/other_user}","gists_url":"https://api.github.com/users/jyasskin/gists{/gist_id}","starred_url":"https://api.github.com/users/jyasskin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jyasskin/subscriptions","organizations_url":"https://api.github.com/users/jyasskin/orgs","repos_url":"https://api.github.com/users/jyasskin/repos","events_url":"https://api.github.com/users/jyasskin/events{/privacy}","received_events_url":"https://api.github.com/users/jyasskin/received_events","type":"User","site_admin":false},"repo":{"id":105045311,"name":"CBORbis","full_name":"jyasskin/CBORbis","owner":{"login":"jyasskin","id":83420,"avatar_url":"https://avatars1.githubusercontent.com/u/83420?v=4","gravatar_id":"","url":"https://api.github.com/users/jyasskin","html_url":"https://github.com/jyasskin","followers_url":"https://api.github.com/users/jyasskin/followers","following_url":"https://api.github.com/users/jyasskin/following{/other_user}","gists_url":"https://api.github.com/users/jyasskin/gists{/gist_id}","starred_url":"https://api.github.com/users/jyasskin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jyasskin/subscriptions","organizations_url":"https://api.github.com/users/jyasskin/orgs","repos_url":"https://api.github.com/users/jyasskin/repos","events_url":"https://api.github.com/users/jyasskin/events{/privacy}","received_events_url":"https://api.github.com/users/jyasskin/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/jyasskin/CBORbis","description":null,"fork":true,"url":"https://api.github.com/repos/jyasskin/CBORbis","forks_url":"https://api.github.com/repos/jyasskin/CBORbis/forks","keys_url":"https://api.github.com/repos/jyasskin/CBORbis/keys{/key_id}","collaborators_url":"https://api.github.com/repos/jyasskin/CBORbis/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/jyasskin/CBORbis/teams","hooks_url":"https://api.github.com/repos/jyasskin/CBORbis/hooks","issue_events_url":"https://api.github.com/repos/jyasskin/CBORbis/issues/events{/number}","events_url":"https://api.github.com/repos/jyasskin/CBORbis/events","assignees_url":"https://api.github.com/repos/jyasskin/CBORbis/assignees{/user}","branches_url":"https://api.github.com/repos/jyasskin/CBORbis/branches{/branch}","tags_url":"https://api.github.com/repos/jyasskin/CBORbis/tags","blobs_url":"https://api.github.com/repos/jyasskin/CBORbis/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/jyasskin/CBORbis/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/jyasskin/CBORbis/git/refs{/sha}","trees_url":"https://api.github.com/repos/jyasskin/CBORbis/git/trees{/sha}","statuses_url":"https://api.github.com/repos/jyasskin/CBORbis/statuses/{sha}","languages_url":"https://api.github.com/repos/jyasskin/CBORbis/languages","stargazers_url":"https://api.github.com/repos/jyasskin/CBORbis/stargazers","contributors_url":"https://api.github.com/repos/jyasskin/CBORbis/contributors","subscribers_url":"https://api.github.com/repos/jyasskin/CBORbis/subscribers","subscription_url":"https://api.github.com/repos/jyasskin/CBORbis/subscription","commits_url":"https://api.github.com/repos/jyasskin/CBORbis/commits{/sha}","git_commits_url":"https://api.github.com/repos/jyasskin/CBORbis/git/commits{/sha}","comments_url":"https://api.github.com/repos/jyasskin/CBORbis/comments{/number}","issue_comment_url":"https://api.github.com/repos/jyasskin/CBORbis/issues/comments{/number}","contents_url":"https://api.github.com/repos/jyasskin/CBORbis/contents/{+path}","compare_url":"https://api.github.com/repos/jyasskin/CBORbis/compare/{base}...{head}","merges_url":"https://api.github.com/repos/jyasskin/CBORbis/merges","archive_url":"https://api.github.com/repos/jyasskin/CBORbis/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/jyasskin/CBORbis/downloads","issues_url":"https://api.github.com/repos/jyasskin/CBORbis/issues{/number}","pulls_url":"https://api.github.com/repos/jyasskin/CBORbis/pulls{/number}","milestones_url":"https://api.github.com/repos/jyasskin/CBORbis/milestones{/number}","notifications_url":"https://api.github.com/repos/jyasskin/CBORbis/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/jyasskin/CBORbis/labels{/name}","releases_url":"https://api.github.com/repos/jyasskin/CBORbis/releases{/id}","deployments_url":"https://api.github.com/repos/jyasskin/CBORbis/deployments","created_at":"2017-09-27T16:59:01Z","updated_at":"2017-09-27T16:59:03Z","pushed_at":"2018-05-25T18:17:45Z","git_url":"git://github.com/jyasskin/CBORbis.git","ssh_url":"git@github.com:jyasskin/CBORbis.git","clone_url":"https://github.com/jyasskin/CBORbis.git","svn_url":"https://github.com/jyasskin/CBORbis","homepage":null,"size":247,"stargazers_count":0,"watchers_count":0,"language":"Makefile","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"other","name":"Other","spdx_id":null,"url":null},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"cbor-wg:master","ref":"master","sha":"9c088101538b365c8327fe8138a23fd47c788c5d","user":{"login":"cbor-wg","id":25103060,"avatar_url":"https://avatars1.githubusercontent.com/u/25103060?v=4","gravatar_id":"","url":"https://api.github.com/users/cbor-wg","html_url":"https://github.com/cbor-wg","followers_url":"https://api.github.com/users/cbor-wg/followers","following_url":"https://api.github.com/users/cbor-wg/following{/other_user}","gists_url":"https://api.github.com/users/cbor-wg/gists{/gist_id}","starred_url":"https://api.github.com/users/cbor-wg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cbor-wg/subscriptions","organizations_url":"https://api.github.com/users/cbor-wg/orgs","repos_url":"https://api.github.com/users/cbor-wg/repos","events_url":"https://api.github.com/users/cbor-wg/events{/privacy}","received_events_url":"https://api.github.com/users/cbor-wg/received_events","type":"Organization","site_admin":false},"repo":{"id":86742073,"name":"CBORbis","full_name":"cbor-wg/CBORbis","owner":{"login":"cbor-wg","id":25103060,"avatar_url":"https://avatars1.githubusercontent.com/u/25103060?v=4","gravatar_id":"","url":"https://api.github.com/users/cbor-wg","html_url":"https://github.com/cbor-wg","followers_url":"https://api.github.com/users/cbor-wg/followers","following_url":"https://api.github.com/users/cbor-wg/following{/other_user}","gists_url":"https://api.github.com/users/cbor-wg/gists{/gist_id}","starred_url":"https://api.github.com/users/cbor-wg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cbor-wg/subscriptions","organizations_url":"https://api.github.com/users/cbor-wg/orgs","repos_url":"https://api.github.com/users/cbor-wg/repos","events_url":"https://api.github.com/users/cbor-wg/events{/privacy}","received_events_url":"https://api.github.com/users/cbor-wg/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/cbor-wg/CBORbis","description":null,"fork":false,"url":"https://api.github.com/repos/cbor-wg/CBORbis","forks_url":"https://api.github.com/repos/cbor-wg/CBORbis/forks","keys_url":"https://api.github.com/repos/cbor-wg/CBORbis/keys{/key_id}","collaborators_url":"https://api.github.com/repos/cbor-wg/CBORbis/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/cbor-wg/CBORbis/teams","hooks_url":"https://api.github.com/repos/cbor-wg/CBORbis/hooks","issue_events_url":"https://api.github.com/repos/cbor-wg/CBORbis/issues/events{/number}","events_url":"https://api.github.com/repos/cbor-wg/CBORbis/events","assignees_url":"https://api.github.com/repos/cbor-wg/CBORbis/assignees{/user}","branches_url":"https://api.github.com/repos/cbor-wg/CBORbis/branches{/branch}","tags_url":"https://api.github.com/repos/cbor-wg/CBORbis/tags","blobs_url":"https://api.github.com/repos/cbor-wg/CBORbis/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/cbor-wg/CBORbis/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/cbor-wg/CBORbis/git/refs{/sha}","trees_url":"https://api.github.com/repos/cbor-wg/CBORbis/git/trees{/sha}","statuses_url":"https://api.github.com/repos/cbor-wg/CBORbis/statuses/{sha}","languages_url":"https://api.github.com/repos/cbor-wg/CBORbis/languages","stargazers_url":"https://api.github.com/repos/cbor-wg/CBORbis/stargazers","contributors_url":"https://api.github.com/repos/cbor-wg/CBORbis/contributors","subscribers_url":"https://api.github.com/repos/cbor-wg/CBORbis/subscribers","subscription_url":"https://api.github.com/repos/cbor-wg/CBORbis/subscription","commits_url":"https://api.github.com/repos/cbor-wg/CBORbis/commits{/sha}","git_commits_url":"https://api.github.com/repos/cbor-wg/CBORbis/git/commits{/sha}","comments_url":"https://api.github.com/repos/cbor-wg/CBORbis/comments{/number}","issue_comment_url":"https://api.github.com/repos/cbor-wg/CBORbis/issues/comments{/number}","contents_url":"https://api.github.com/repos/cbor-wg/CBORbis/contents/{+path}","compare_url":"https://api.github.com/repos/cbor-wg/CBORbis/compare/{base}...{head}","merges_url":"https://api.github.com/repos/cbor-wg/CBORbis/merges","archive_url":"https://api.github.com/repos/cbor-wg/CBORbis/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/cbor-wg/CBORbis/downloads","issues_url":"https://api.github.com/repos/cbor-wg/CBORbis/issues{/number}","pulls_url":"https://api.github.com/repos/cbor-wg/CBORbis/pulls{/number}","milestones_url":"https://api.github.com/repos/cbor-wg/CBORbis/milestones{/number}","notifications_url":"https://api.github.com/repos/cbor-wg/CBORbis/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/cbor-wg/CBORbis/labels{/name}","releases_url":"https://api.github.com/repos/cbor-wg/CBORbis/releases{/id}","deployments_url":"https://api.github.com/repos/cbor-wg/CBORbis/deployments","created_at":"2017-03-30T19:45:41Z","updated_at":"2018-04-11T20:41:59Z","pushed_at":"2018-05-25T18:17:46Z","git_url":"git://github.com/cbor-wg/CBORbis.git","ssh_url":"git@github.com:cbor-wg/CBORbis.git","clone_url":"https://github.com/cbor-wg/CBORbis.git","svn_url":"https://github.com/cbor-wg/CBORbis","homepage":null,"size":295,"stargazers_count":2,"watchers_count":2,"language":"Makefile","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":6,"mirror_url":null,"archived":false,"open_issues_count":14,"license":{"key":"other","name":"Other","spdx_id":null,"url":null},"forks":6,"open_issues":14,"watchers":2,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/20"},"html":{"href":"https://github.com/cbor-wg/CBORbis/pull/20"},"issue":{"href":"https://api.github.com/repos/cbor-wg/CBORbis/issues/20"},"comments":{"href":"https://api.github.com/repos/cbor-wg/CBORbis/issues/20/comments"},"review_comments":{"href":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/20/comments"},"review_comment":{"href":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/cbor-wg/CBORbis/pulls/20/commits"},"statuses":{"href":"https://api.github.com/repos/cbor-wg/CBORbis/statuses/545dbe14548b65c5e7e831b116f686150f58f6d1"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"BurtHarris"}}