{"action":"created","comment":{"url":"https://api.github.com/repos/dotnet/coreclr/pulls/comments/74637976","id":74637976,"diff_hunk":"@@ -352,19 +197,7 @@ void record_changed_seg (uint8_t* start, uint8_t* end,\n void record_global_mechanism (int mech_index);\n #endif //GC_CONFIG_DRIVEN\n \n-//constants for the flags parameter to the gc call back\n-\n-#define GC_CALL_INTERIOR            0x1\n-#define GC_CALL_PINNED              0x2\n-#define GC_CALL_CHECK_APP_DOMAIN    0x4\n-\n-//flags for GCHeap::Alloc(...)\n-#define GC_ALLOC_FINALIZE 0x1\n-#define GC_ALLOC_CONTAINS_REF 0x2\n-#define GC_ALLOC_ALIGN8_BIAS 0x4\n-#define GC_ALLOC_ALIGN8 0x8\n-\n-class GCHeap {\n+class GCHeap : public IGCHeap {","path":"src/gc/gc.h","position":251,"original_position":251,"commit_id":"647aec1eeb7fa4a80310c995377250a89be9b395","original_commit_id":"647aec1eeb7fa4a80310c995377250a89be9b395","user":{"login":"swgillespie","id":1871912,"avatar_url":"https://avatars.githubusercontent.com/u/1871912?v=3","gravatar_id":"","url":"https://api.github.com/users/swgillespie","html_url":"https://github.com/swgillespie","followers_url":"https://api.github.com/users/swgillespie/followers","following_url":"https://api.github.com/users/swgillespie/following{/other_user}","gists_url":"https://api.github.com/users/swgillespie/gists{/gist_id}","starred_url":"https://api.github.com/users/swgillespie/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swgillespie/subscriptions","organizations_url":"https://api.github.com/users/swgillespie/orgs","repos_url":"https://api.github.com/users/swgillespie/repos","events_url":"https://api.github.com/users/swgillespie/events{/privacy}","received_events_url":"https://api.github.com/users/swgillespie/received_events","type":"User","site_admin":false},"body":"Will do!","created_at":"2016-08-12T18:25:10Z","updated_at":"2016-08-12T18:25:10Z","html_url":"https://github.com/dotnet/coreclr/pull/6693#discussion_r74637976","pull_request_url":"https://api.github.com/repos/dotnet/coreclr/pulls/6693","_links":{"self":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/comments/74637976"},"html":{"href":"https://github.com/dotnet/coreclr/pull/6693#discussion_r74637976"},"pull_request":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/6693"}}},"pull_request":{"url":"https://api.github.com/repos/dotnet/coreclr/pulls/6693","id":80882678,"html_url":"https://github.com/dotnet/coreclr/pull/6693","diff_url":"https://github.com/dotnet/coreclr/pull/6693.diff","patch_url":"https://github.com/dotnet/coreclr/pull/6693.patch","issue_url":"https://api.github.com/repos/dotnet/coreclr/issues/6693","number":6693,"state":"open","locked":false,"title":"[WIP] Move the GC behind an interface and use that interface in the VM","user":{"login":"swgillespie","id":1871912,"avatar_url":"https://avatars.githubusercontent.com/u/1871912?v=3","gravatar_id":"","url":"https://api.github.com/users/swgillespie","html_url":"https://github.com/swgillespie","followers_url":"https://api.github.com/users/swgillespie/followers","following_url":"https://api.github.com/users/swgillespie/following{/other_user}","gists_url":"https://api.github.com/users/swgillespie/gists{/gist_id}","starred_url":"https://api.github.com/users/swgillespie/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swgillespie/subscriptions","organizations_url":"https://api.github.com/users/swgillespie/orgs","repos_url":"https://api.github.com/users/swgillespie/repos","events_url":"https://api.github.com/users/swgillespie/events{/privacy}","received_events_url":"https://api.github.com/users/swgillespie/received_events","type":"User","site_admin":false},"body":"This is quite a large change that introduces a new interface `IGCHeap` that abstracts away the details of the GC away from the VM. The changes in this PR fall into three categories:\r\n\r\n1. The definition of `IGCHeap` itself (`gcinterface.h`) and code motion from `gc.h` to `gcinterface.h`. The public interface of the GC was moved from `gc.h` to `gcinterface.h`. This includes a number of enums and macro definitions that were used throughout the VM.\r\n2. The abstraction of `alloc_context` (`gcinterface.h`). `alloc_context` is embedded by-value into the `Thread` class and so its layout must be known to the VM at compile-time. However, `alloc_context` also contains implementation details of the GC that can't be exposed by the interface. To solve this problem, `gc_alloc_context` is defined in `gcinterface.h` that provides the VM-facing interface of `alloc_context` (including two void pointers, containing GC implementation details). `alloc_context` is defined in `gc.h`, which contains two inline methods that reinterpret the opaque fields in `gc_alloc_context` as the values that the GC expects to be there today. This change required renaming VM uses of `alloc_context` to `gc_alloc_context`, as well as requiring a few downcasts within the GC itself whenever the GC-specific fields on `gc_alloc_context` are used. This accounts for the majority of the changes within `gc.cpp`.\r\n3. Changing uses of `GCHeap` to `IGCHeap`. The VM today refers to the GC using the `GCHeap` class, so the act of introducing an interface naturally required uses of `GCHeap` to be renamed to `IGCHeap`.\r\n\r\nWhile doing this work, I encountered six GC interface violations that I have left in place:\r\n\r\n* [weakreference.cpp](https://github.com/swgillespie/coreclr/blob/gc-interface/src/vm/weakreferencenative.cpp#L16)\r\n* [marshalnative.cpp](https://github.com/swgillespie/coreclr/blob/gc-interface/src/vm/marshalnative.cpp#L42)\r\n* [appdomain.cpp 1](https://github.com/swgillespie/coreclr/blob/gc-interface/src/vm/appdomain.cpp#L5041)\r\n* [appdomain.cpp 2](https://github.com/swgillespie/coreclr/blob/gc-interface/src/vm/appdomain.cpp#L5141)\r\n* [appdomain.cpp 3](https://github.com/swgillespie/coreclr/blob/gc-interface/src/vm/appdomain.cpp#L5467)\r\n* [appdomain.cpp 4](https://github.com/swgillespie/coreclr/blob/gc-interface/src/vm/appdomain.cpp#L11568)\r\n\r\nAll of these files directly reference private details of the handle table. We will need to address these at some point. All of the uses within `appdomain.cpp` are for debug asserts, but the uses within `weakreference.cpp` and `marshaknative.cpp` are not.\r\n\r\nOnce this has been merged, the next steps will be to ensure that the GC always interacts with the EE and OS through `GCToEEInterface` and `GCToOSInterface`, respectively. From there, we can decouple the GC from VM internals. Another step that will need to be done (not necessarily before or after the previous step) will be to trim down `IGCHeap` until it presents the interface that we'd like to exist, rather than how it is today.\r\n\r\nI welcome any and all comments on this change. This is the start of a large change to the GC and VM as a whole, so I welcome all feedback to ensure that we are doing this in the most efficient and clean way. I do not plan on merging this until we are all happy with the direction and implications of this change.\r\n","created_at":"2016-08-10T23:04:11Z","updated_at":"2016-08-12T18:25:10Z","closed_at":null,"merged_at":null,"merge_commit_sha":"f8eedc0d0902b1e80ef6e4d57616a9557997e769","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/dotnet/coreclr/pulls/6693/commits","review_comments_url":"https://api.github.com/repos/dotnet/coreclr/pulls/6693/comments","review_comment_url":"https://api.github.com/repos/dotnet/coreclr/pulls/comments{/number}","comments_url":"https://api.github.com/repos/dotnet/coreclr/issues/6693/comments","statuses_url":"https://api.github.com/repos/dotnet/coreclr/statuses/647aec1eeb7fa4a80310c995377250a89be9b395","head":{"label":"swgillespie:gc-interface","ref":"gc-interface","sha":"647aec1eeb7fa4a80310c995377250a89be9b395","user":{"login":"swgillespie","id":1871912,"avatar_url":"https://avatars.githubusercontent.com/u/1871912?v=3","gravatar_id":"","url":"https://api.github.com/users/swgillespie","html_url":"https://github.com/swgillespie","followers_url":"https://api.github.com/users/swgillespie/followers","following_url":"https://api.github.com/users/swgillespie/following{/other_user}","gists_url":"https://api.github.com/users/swgillespie/gists{/gist_id}","starred_url":"https://api.github.com/users/swgillespie/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swgillespie/subscriptions","organizations_url":"https://api.github.com/users/swgillespie/orgs","repos_url":"https://api.github.com/users/swgillespie/repos","events_url":"https://api.github.com/users/swgillespie/events{/privacy}","received_events_url":"https://api.github.com/users/swgillespie/received_events","type":"User","site_admin":false},"repo":{"id":47656398,"name":"coreclr","full_name":"swgillespie/coreclr","owner":{"login":"swgillespie","id":1871912,"avatar_url":"https://avatars.githubusercontent.com/u/1871912?v=3","gravatar_id":"","url":"https://api.github.com/users/swgillespie","html_url":"https://github.com/swgillespie","followers_url":"https://api.github.com/users/swgillespie/followers","following_url":"https://api.github.com/users/swgillespie/following{/other_user}","gists_url":"https://api.github.com/users/swgillespie/gists{/gist_id}","starred_url":"https://api.github.com/users/swgillespie/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swgillespie/subscriptions","organizations_url":"https://api.github.com/users/swgillespie/orgs","repos_url":"https://api.github.com/users/swgillespie/repos","events_url":"https://api.github.com/users/swgillespie/events{/privacy}","received_events_url":"https://api.github.com/users/swgillespie/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/swgillespie/coreclr","description":"This repo contains the .NET Core runtime, called CoreCLR, and the base library, called mscorlib. It includes the garbage collector, JIT compiler, base .NET data types and many low-level classes.","fork":true,"url":"https://api.github.com/repos/swgillespie/coreclr","forks_url":"https://api.github.com/repos/swgillespie/coreclr/forks","keys_url":"https://api.github.com/repos/swgillespie/coreclr/keys{/key_id}","collaborators_url":"https://api.github.com/repos/swgillespie/coreclr/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/swgillespie/coreclr/teams","hooks_url":"https://api.github.com/repos/swgillespie/coreclr/hooks","issue_events_url":"https://api.github.com/repos/swgillespie/coreclr/issues/events{/number}","events_url":"https://api.github.com/repos/swgillespie/coreclr/events","assignees_url":"https://api.github.com/repos/swgillespie/coreclr/assignees{/user}","branches_url":"https://api.github.com/repos/swgillespie/coreclr/branches{/branch}","tags_url":"https://api.github.com/repos/swgillespie/coreclr/tags","blobs_url":"https://api.github.com/repos/swgillespie/coreclr/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/swgillespie/coreclr/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/swgillespie/coreclr/git/refs{/sha}","trees_url":"https://api.github.com/repos/swgillespie/coreclr/git/trees{/sha}","statuses_url":"https://api.github.com/repos/swgillespie/coreclr/statuses/{sha}","languages_url":"https://api.github.com/repos/swgillespie/coreclr/languages","stargazers_url":"https://api.github.com/repos/swgillespie/coreclr/stargazers","contributors_url":"https://api.github.com/repos/swgillespie/coreclr/contributors","subscribers_url":"https://api.github.com/repos/swgillespie/coreclr/subscribers","subscription_url":"https://api.github.com/repos/swgillespie/coreclr/subscription","commits_url":"https://api.github.com/repos/swgillespie/coreclr/commits{/sha}","git_commits_url":"https://api.github.com/repos/swgillespie/coreclr/git/commits{/sha}","comments_url":"https://api.github.com/repos/swgillespie/coreclr/comments{/number}","issue_comment_url":"https://api.github.com/repos/swgillespie/coreclr/issues/comments{/number}","contents_url":"https://api.github.com/repos/swgillespie/coreclr/contents/{+path}","compare_url":"https://api.github.com/repos/swgillespie/coreclr/compare/{base}...{head}","merges_url":"https://api.github.com/repos/swgillespie/coreclr/merges","archive_url":"https://api.github.com/repos/swgillespie/coreclr/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/swgillespie/coreclr/downloads","issues_url":"https://api.github.com/repos/swgillespie/coreclr/issues{/number}","pulls_url":"https://api.github.com/repos/swgillespie/coreclr/pulls{/number}","milestones_url":"https://api.github.com/repos/swgillespie/coreclr/milestones{/number}","notifications_url":"https://api.github.com/repos/swgillespie/coreclr/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/swgillespie/coreclr/labels{/name}","releases_url":"https://api.github.com/repos/swgillespie/coreclr/releases{/id}","deployments_url":"https://api.github.com/repos/swgillespie/coreclr/deployments","created_at":"2015-12-08T23:40:42Z","updated_at":"2016-01-13T23:23:53Z","pushed_at":"2016-08-12T00:50:59Z","git_url":"git://github.com/swgillespie/coreclr.git","ssh_url":"git@github.com:swgillespie/coreclr.git","clone_url":"https://github.com/swgillespie/coreclr.git","svn_url":"https://github.com/swgillespie/coreclr","homepage":"http://dotnet.github.io/","size":103018,"stargazers_count":0,"watchers_count":0,"language":"C#","has_issues":false,"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":"dotnet:master","ref":"master","sha":"4511b187eb6ec6347d8f8b841dc53982fa1d5884","user":{"login":"dotnet","id":9141961,"avatar_url":"https://avatars.githubusercontent.com/u/9141961?v=3","gravatar_id":"","url":"https://api.github.com/users/dotnet","html_url":"https://github.com/dotnet","followers_url":"https://api.github.com/users/dotnet/followers","following_url":"https://api.github.com/users/dotnet/following{/other_user}","gists_url":"https://api.github.com/users/dotnet/gists{/gist_id}","starred_url":"https://api.github.com/users/dotnet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dotnet/subscriptions","organizations_url":"https://api.github.com/users/dotnet/orgs","repos_url":"https://api.github.com/users/dotnet/repos","events_url":"https://api.github.com/users/dotnet/events{/privacy}","received_events_url":"https://api.github.com/users/dotnet/received_events","type":"Organization","site_admin":false},"repo":{"id":30092893,"name":"coreclr","full_name":"dotnet/coreclr","owner":{"login":"dotnet","id":9141961,"avatar_url":"https://avatars.githubusercontent.com/u/9141961?v=3","gravatar_id":"","url":"https://api.github.com/users/dotnet","html_url":"https://github.com/dotnet","followers_url":"https://api.github.com/users/dotnet/followers","following_url":"https://api.github.com/users/dotnet/following{/other_user}","gists_url":"https://api.github.com/users/dotnet/gists{/gist_id}","starred_url":"https://api.github.com/users/dotnet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dotnet/subscriptions","organizations_url":"https://api.github.com/users/dotnet/orgs","repos_url":"https://api.github.com/users/dotnet/repos","events_url":"https://api.github.com/users/dotnet/events{/privacy}","received_events_url":"https://api.github.com/users/dotnet/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/dotnet/coreclr","description":"This repo contains the .NET Core runtime, called CoreCLR, and the base library, called mscorlib. It includes the garbage collector, JIT compiler, base .NET data types and many low-level classes.","fork":false,"url":"https://api.github.com/repos/dotnet/coreclr","forks_url":"https://api.github.com/repos/dotnet/coreclr/forks","keys_url":"https://api.github.com/repos/dotnet/coreclr/keys{/key_id}","collaborators_url":"https://api.github.com/repos/dotnet/coreclr/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/dotnet/coreclr/teams","hooks_url":"https://api.github.com/repos/dotnet/coreclr/hooks","issue_events_url":"https://api.github.com/repos/dotnet/coreclr/issues/events{/number}","events_url":"https://api.github.com/repos/dotnet/coreclr/events","assignees_url":"https://api.github.com/repos/dotnet/coreclr/assignees{/user}","branches_url":"https://api.github.com/repos/dotnet/coreclr/branches{/branch}","tags_url":"https://api.github.com/repos/dotnet/coreclr/tags","blobs_url":"https://api.github.com/repos/dotnet/coreclr/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/dotnet/coreclr/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/dotnet/coreclr/git/refs{/sha}","trees_url":"https://api.github.com/repos/dotnet/coreclr/git/trees{/sha}","statuses_url":"https://api.github.com/repos/dotnet/coreclr/statuses/{sha}","languages_url":"https://api.github.com/repos/dotnet/coreclr/languages","stargazers_url":"https://api.github.com/repos/dotnet/coreclr/stargazers","contributors_url":"https://api.github.com/repos/dotnet/coreclr/contributors","subscribers_url":"https://api.github.com/repos/dotnet/coreclr/subscribers","subscription_url":"https://api.github.com/repos/dotnet/coreclr/subscription","commits_url":"https://api.github.com/repos/dotnet/coreclr/commits{/sha}","git_commits_url":"https://api.github.com/repos/dotnet/coreclr/git/commits{/sha}","comments_url":"https://api.github.com/repos/dotnet/coreclr/comments{/number}","issue_comment_url":"https://api.github.com/repos/dotnet/coreclr/issues/comments{/number}","contents_url":"https://api.github.com/repos/dotnet/coreclr/contents/{+path}","compare_url":"https://api.github.com/repos/dotnet/coreclr/compare/{base}...{head}","merges_url":"https://api.github.com/repos/dotnet/coreclr/merges","archive_url":"https://api.github.com/repos/dotnet/coreclr/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/dotnet/coreclr/downloads","issues_url":"https://api.github.com/repos/dotnet/coreclr/issues{/number}","pulls_url":"https://api.github.com/repos/dotnet/coreclr/pulls{/number}","milestones_url":"https://api.github.com/repos/dotnet/coreclr/milestones{/number}","notifications_url":"https://api.github.com/repos/dotnet/coreclr/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/dotnet/coreclr/labels{/name}","releases_url":"https://api.github.com/repos/dotnet/coreclr/releases{/id}","deployments_url":"https://api.github.com/repos/dotnet/coreclr/deployments","created_at":"2015-01-30T22:10:02Z","updated_at":"2016-08-12T16:02:01Z","pushed_at":"2016-08-12T18:09:07Z","git_url":"git://github.com/dotnet/coreclr.git","ssh_url":"git@github.com:dotnet/coreclr.git","clone_url":"https://github.com/dotnet/coreclr.git","svn_url":"https://github.com/dotnet/coreclr","homepage":"https://docs.microsoft.com/dotnet/","size":114875,"stargazers_count":6775,"watchers_count":6775,"language":"C#","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1597,"mirror_url":null,"open_issues_count":918,"forks":1597,"open_issues":918,"watchers":6775,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/6693"},"html":{"href":"https://github.com/dotnet/coreclr/pull/6693"},"issue":{"href":"https://api.github.com/repos/dotnet/coreclr/issues/6693"},"comments":{"href":"https://api.github.com/repos/dotnet/coreclr/issues/6693/comments"},"review_comments":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/6693/comments"},"review_comment":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/6693/commits"},"statuses":{"href":"https://api.github.com/repos/dotnet/coreclr/statuses/647aec1eeb7fa4a80310c995377250a89be9b395"}}}}
{"actor":{"display_login":"swgillespie"}}