{"action":"created","comment":{"url":"https://api.github.com/repos/dropbox/pyston/pulls/comments/81440555","pull_request_review_id":2422328,"id":81440555,"diff_hunk":"@@ -1270,6 +1271,8 @@ class BoxedModule : public Box {\n public:\n     DEFAULT_CLASS(module_cls);\n \n+    std::vector<Box*> constants;","path":"src/runtime/types.h","position":12,"original_position":12,"commit_id":"6f7d44fd48a1e124514e46250233678d83392cd4","original_commit_id":"6f7d44fd48a1e124514e46250233678d83392cd4","user":{"login":"kmod","id":271916,"avatar_url":"https://avatars.githubusercontent.com/u/271916?v=3","gravatar_id":"","url":"https://api.github.com/users/kmod","html_url":"https://github.com/kmod","followers_url":"https://api.github.com/users/kmod/followers","following_url":"https://api.github.com/users/kmod/following{/other_user}","gists_url":"https://api.github.com/users/kmod/gists{/gist_id}","starred_url":"https://api.github.com/users/kmod/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kmod/subscriptions","organizations_url":"https://api.github.com/users/kmod/orgs","repos_url":"https://api.github.com/users/kmod/repos","events_url":"https://api.github.com/users/kmod/events{/privacy}","received_events_url":"https://api.github.com/users/kmod/received_events","type":"User","site_admin":false},"body":"I think this is worth a comment here -- I wasn't expecting to find that constants are stored twice on the module, once in the `str/int/long/etc_constants` and once in `constants`.  I think we should eventually move to having the constants be stored on the code objects, but for now I think the current way is fine but should have a comment / todo.","created_at":"2016-10-01T01:42:55Z","updated_at":"2016-10-01T01:45:12Z","html_url":"https://github.com/dropbox/pyston/pull/1377#discussion_r81440555","pull_request_url":"https://api.github.com/repos/dropbox/pyston/pulls/1377","_links":{"self":{"href":"https://api.github.com/repos/dropbox/pyston/pulls/comments/81440555"},"html":{"href":"https://github.com/dropbox/pyston/pull/1377#discussion_r81440555"},"pull_request":{"href":"https://api.github.com/repos/dropbox/pyston/pulls/1377"}}},"pull_request":{"url":"https://api.github.com/repos/dropbox/pyston/pulls/1377","id":86967891,"html_url":"https://github.com/dropbox/pyston/pull/1377","diff_url":"https://github.com/dropbox/pyston/pull/1377.diff","patch_url":"https://github.com/dropbox/pyston/pull/1377.patch","issue_url":"https://api.github.com/repos/dropbox/pyston/issues/1377","number":1377,"state":"open","locked":false,"title":"wip: bst: convert all nodes to directly operate at vregs instead of names","user":{"login":"undingen","id":7397630,"avatar_url":"https://avatars.githubusercontent.com/u/7397630?v=3","gravatar_id":"","url":"https://api.github.com/users/undingen","html_url":"https://github.com/undingen","followers_url":"https://api.github.com/users/undingen/followers","following_url":"https://api.github.com/users/undingen/following{/other_user}","gists_url":"https://api.github.com/users/undingen/gists{/gist_id}","starred_url":"https://api.github.com/users/undingen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/undingen/subscriptions","organizations_url":"https://api.github.com/users/undingen/orgs","repos_url":"https://api.github.com/users/undingen/repos","events_url":"https://api.github.com/users/undingen/events{/privacy}","received_events_url":"https://api.github.com/users/undingen/received_events","type":"User","site_admin":false},"body":"**basic design:**\r\nThis PR changes our BST nodes to directly operate on vregs instead of pointers to other nodes and names (except a few exceptions: `BST_Invoke`, `BST_MakeFunction` and `BST_MakeClass` which still needs to get converted).\r\nMost nodes got a destination vreg and one or more source vregs. Currently all of them are 32bit long but I plan to store them more compact very soon. Some nodes support a variable size of operands (e.g. the tuple node) but the size can't change after creating the node. I removed several unneeded opcodes and split a lot of nodes into separate opcodes (it may make sense to split them even further in the future). \r\nGenerally all instructions except `CopyVReg` kill the source operand vregs except if the source is a ref to a constant. If one needs the preserve the source vreg on needs to create a new temporary using the `CopyVReg` opcode.\r\n\r\nThere is a special vreg number: `VREG_UNDEFINED = std::numeric_limits<int>::min()`.\r\n- when it's set as an operand vreg: it means that this is a not-set optional argument. (e.g. for a slice which only has `lower` set, `upper` would be `VREG_UNDEFINED`)\r\n- if it's the destination it's means the result value should get immediately killed (e.g. `invoke 15 16: %undef = %11(%14)` this is a call whose result gets ignored)\r\n\r\nall other negative vreg numbers are indices into a constant table (after adding 1 and making them positive).\r\n(e.g. `(4, 2, 'lala')` generates:  `%undef = (%-1|4|, %-2|2|, %-3|'lala'|)` this creates a tuple whose elements are the constant idx -1, -2 and -3. In order to make it easier for a human to understand we print the actual value of the constant between | characters)\r\n- constants can be all str and numeric types and 'None'.\r\n- every constant will only get stored once in the table\r\n\r\nthis reduces the total memory usage by about 20% currently but I'm very sure with the future changes it will be significantly lower.\r\n\r\n**near future:**\r\n- change the jump and branch instruction to reference `CFGBlocks` by index.\r\n- store all `InternedString` inside a table and use indices into the the table to access them.\r\n- remove the `BoxedCode*` member\r\n- devirtualize the classes\r\n= with this changes the bytecode can get freely copied around (only need to update the CFGBlock table) which allows us to store them directly next to each other.\r\n\r\n- I plan to use one bit of the the opcode to mark the instruction as only requiring 8bit vreg operands (which should handle the majority of cases with 128 temps and 127 constants + 1undef vreg value) \r\n- another bit will get used to specify if this instruction is inside an `invoke`. if this bit is set there are two 1 or 4 bytes long block indices directly behind the instruction.\r\n \r\n- serialize the bytecode to disk. (maybe serialize the constant table using pickle)\r\n\r\n**thing which need to get improved**\r\n- currently the constant table get's attached to the `BoxedModule` maybe there is a better location, I also needed to pass the `BoxedModule` into some functions e.g. BST printing because otherwise we could not pretty-print the constants\r\n- `BST_Name` is not an opcode it's just used to initialize the arguments when a function get's called and stores where and how the arguments need to get stored.\r\n- more consistent opcode names and rename `TmpValue` to something better\r\n- we currently don't print the `InternedString` name - we only print the vreg number\r\n\r\n**additional changed made which are hidden in the large diff** :-1:\r\n- removed unused code initializing the items of `BST_Dict` (we use/used separate assignments to add the items)\r\n- lower `ExtSlice` inside the CFG phase to a tuple of slices\r\n- separated opcode for load subscript when it needs to be a slice and when it's only lower and upper (=`__getslice__`) before this got handled in the interpreter/jit\r\n- generate a constant `None` load inside the CFG when `None` gets loaded by name","created_at":"2016-09-27T16:45:48Z","updated_at":"2016-10-01T01:45:12Z","closed_at":null,"merged_at":null,"merge_commit_sha":"f9ef1b517ef02b70de06a5fd5d961701b9fadfc9","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/dropbox/pyston/pulls/1377/commits","review_comments_url":"https://api.github.com/repos/dropbox/pyston/pulls/1377/comments","review_comment_url":"https://api.github.com/repos/dropbox/pyston/pulls/comments{/number}","comments_url":"https://api.github.com/repos/dropbox/pyston/issues/1377/comments","statuses_url":"https://api.github.com/repos/dropbox/pyston/statuses/6f7d44fd48a1e124514e46250233678d83392cd4","head":{"label":"undingen:new_bst4","ref":"new_bst4","sha":"6f7d44fd48a1e124514e46250233678d83392cd4","user":{"login":"undingen","id":7397630,"avatar_url":"https://avatars.githubusercontent.com/u/7397630?v=3","gravatar_id":"","url":"https://api.github.com/users/undingen","html_url":"https://github.com/undingen","followers_url":"https://api.github.com/users/undingen/followers","following_url":"https://api.github.com/users/undingen/following{/other_user}","gists_url":"https://api.github.com/users/undingen/gists{/gist_id}","starred_url":"https://api.github.com/users/undingen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/undingen/subscriptions","organizations_url":"https://api.github.com/users/undingen/orgs","repos_url":"https://api.github.com/users/undingen/repos","events_url":"https://api.github.com/users/undingen/events{/privacy}","received_events_url":"https://api.github.com/users/undingen/received_events","type":"User","site_admin":false},"repo":{"id":19117126,"name":"pyston","full_name":"undingen/pyston","owner":{"login":"undingen","id":7397630,"avatar_url":"https://avatars.githubusercontent.com/u/7397630?v=3","gravatar_id":"","url":"https://api.github.com/users/undingen","html_url":"https://github.com/undingen","followers_url":"https://api.github.com/users/undingen/followers","following_url":"https://api.github.com/users/undingen/following{/other_user}","gists_url":"https://api.github.com/users/undingen/gists{/gist_id}","starred_url":"https://api.github.com/users/undingen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/undingen/subscriptions","organizations_url":"https://api.github.com/users/undingen/orgs","repos_url":"https://api.github.com/users/undingen/repos","events_url":"https://api.github.com/users/undingen/events{/privacy}","received_events_url":"https://api.github.com/users/undingen/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/undingen/pyston","description":"An open-source Python implementation using JIT techniques. ","fork":true,"url":"https://api.github.com/repos/undingen/pyston","forks_url":"https://api.github.com/repos/undingen/pyston/forks","keys_url":"https://api.github.com/repos/undingen/pyston/keys{/key_id}","collaborators_url":"https://api.github.com/repos/undingen/pyston/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/undingen/pyston/teams","hooks_url":"https://api.github.com/repos/undingen/pyston/hooks","issue_events_url":"https://api.github.com/repos/undingen/pyston/issues/events{/number}","events_url":"https://api.github.com/repos/undingen/pyston/events","assignees_url":"https://api.github.com/repos/undingen/pyston/assignees{/user}","branches_url":"https://api.github.com/repos/undingen/pyston/branches{/branch}","tags_url":"https://api.github.com/repos/undingen/pyston/tags","blobs_url":"https://api.github.com/repos/undingen/pyston/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/undingen/pyston/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/undingen/pyston/git/refs{/sha}","trees_url":"https://api.github.com/repos/undingen/pyston/git/trees{/sha}","statuses_url":"https://api.github.com/repos/undingen/pyston/statuses/{sha}","languages_url":"https://api.github.com/repos/undingen/pyston/languages","stargazers_url":"https://api.github.com/repos/undingen/pyston/stargazers","contributors_url":"https://api.github.com/repos/undingen/pyston/contributors","subscribers_url":"https://api.github.com/repos/undingen/pyston/subscribers","subscription_url":"https://api.github.com/repos/undingen/pyston/subscription","commits_url":"https://api.github.com/repos/undingen/pyston/commits{/sha}","git_commits_url":"https://api.github.com/repos/undingen/pyston/git/commits{/sha}","comments_url":"https://api.github.com/repos/undingen/pyston/comments{/number}","issue_comment_url":"https://api.github.com/repos/undingen/pyston/issues/comments{/number}","contents_url":"https://api.github.com/repos/undingen/pyston/contents/{+path}","compare_url":"https://api.github.com/repos/undingen/pyston/compare/{base}...{head}","merges_url":"https://api.github.com/repos/undingen/pyston/merges","archive_url":"https://api.github.com/repos/undingen/pyston/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/undingen/pyston/downloads","issues_url":"https://api.github.com/repos/undingen/pyston/issues{/number}","pulls_url":"https://api.github.com/repos/undingen/pyston/pulls{/number}","milestones_url":"https://api.github.com/repos/undingen/pyston/milestones{/number}","notifications_url":"https://api.github.com/repos/undingen/pyston/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/undingen/pyston/labels{/name}","releases_url":"https://api.github.com/repos/undingen/pyston/releases{/id}","deployments_url":"https://api.github.com/repos/undingen/pyston/deployments","created_at":"2014-04-24T17:17:10Z","updated_at":"2015-01-27T22:17:13Z","pushed_at":"2016-09-30T15:11:46Z","git_url":"git://github.com/undingen/pyston.git","ssh_url":"git@github.com:undingen/pyston.git","clone_url":"https://github.com/undingen/pyston.git","svn_url":"https://github.com/undingen/pyston","homepage":"https://tech.dropbox.com/2014/04/introducing-pyston-an-upcoming-jit-based-python-implementation/","size":42143,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":false,"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":"dropbox:master","ref":"master","sha":"4e10a4f077a3fa5d59906b792b8dc4f55cbb1df8","user":{"login":"dropbox","id":559357,"avatar_url":"https://avatars.githubusercontent.com/u/559357?v=3","gravatar_id":"","url":"https://api.github.com/users/dropbox","html_url":"https://github.com/dropbox","followers_url":"https://api.github.com/users/dropbox/followers","following_url":"https://api.github.com/users/dropbox/following{/other_user}","gists_url":"https://api.github.com/users/dropbox/gists{/gist_id}","starred_url":"https://api.github.com/users/dropbox/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dropbox/subscriptions","organizations_url":"https://api.github.com/users/dropbox/orgs","repos_url":"https://api.github.com/users/dropbox/repos","events_url":"https://api.github.com/users/dropbox/events{/privacy}","received_events_url":"https://api.github.com/users/dropbox/received_events","type":"Organization","site_admin":false},"repo":{"id":18320947,"name":"pyston","full_name":"dropbox/pyston","owner":{"login":"dropbox","id":559357,"avatar_url":"https://avatars.githubusercontent.com/u/559357?v=3","gravatar_id":"","url":"https://api.github.com/users/dropbox","html_url":"https://github.com/dropbox","followers_url":"https://api.github.com/users/dropbox/followers","following_url":"https://api.github.com/users/dropbox/following{/other_user}","gists_url":"https://api.github.com/users/dropbox/gists{/gist_id}","starred_url":"https://api.github.com/users/dropbox/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dropbox/subscriptions","organizations_url":"https://api.github.com/users/dropbox/orgs","repos_url":"https://api.github.com/users/dropbox/repos","events_url":"https://api.github.com/users/dropbox/events{/privacy}","received_events_url":"https://api.github.com/users/dropbox/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/dropbox/pyston","description":"An open-source Python implementation using JIT techniques. ","fork":false,"url":"https://api.github.com/repos/dropbox/pyston","forks_url":"https://api.github.com/repos/dropbox/pyston/forks","keys_url":"https://api.github.com/repos/dropbox/pyston/keys{/key_id}","collaborators_url":"https://api.github.com/repos/dropbox/pyston/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/dropbox/pyston/teams","hooks_url":"https://api.github.com/repos/dropbox/pyston/hooks","issue_events_url":"https://api.github.com/repos/dropbox/pyston/issues/events{/number}","events_url":"https://api.github.com/repos/dropbox/pyston/events","assignees_url":"https://api.github.com/repos/dropbox/pyston/assignees{/user}","branches_url":"https://api.github.com/repos/dropbox/pyston/branches{/branch}","tags_url":"https://api.github.com/repos/dropbox/pyston/tags","blobs_url":"https://api.github.com/repos/dropbox/pyston/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/dropbox/pyston/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/dropbox/pyston/git/refs{/sha}","trees_url":"https://api.github.com/repos/dropbox/pyston/git/trees{/sha}","statuses_url":"https://api.github.com/repos/dropbox/pyston/statuses/{sha}","languages_url":"https://api.github.com/repos/dropbox/pyston/languages","stargazers_url":"https://api.github.com/repos/dropbox/pyston/stargazers","contributors_url":"https://api.github.com/repos/dropbox/pyston/contributors","subscribers_url":"https://api.github.com/repos/dropbox/pyston/subscribers","subscription_url":"https://api.github.com/repos/dropbox/pyston/subscription","commits_url":"https://api.github.com/repos/dropbox/pyston/commits{/sha}","git_commits_url":"https://api.github.com/repos/dropbox/pyston/git/commits{/sha}","comments_url":"https://api.github.com/repos/dropbox/pyston/comments{/number}","issue_comment_url":"https://api.github.com/repos/dropbox/pyston/issues/comments{/number}","contents_url":"https://api.github.com/repos/dropbox/pyston/contents/{+path}","compare_url":"https://api.github.com/repos/dropbox/pyston/compare/{base}...{head}","merges_url":"https://api.github.com/repos/dropbox/pyston/merges","archive_url":"https://api.github.com/repos/dropbox/pyston/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/dropbox/pyston/downloads","issues_url":"https://api.github.com/repos/dropbox/pyston/issues{/number}","pulls_url":"https://api.github.com/repos/dropbox/pyston/pulls{/number}","milestones_url":"https://api.github.com/repos/dropbox/pyston/milestones{/number}","notifications_url":"https://api.github.com/repos/dropbox/pyston/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/dropbox/pyston/labels{/name}","releases_url":"https://api.github.com/repos/dropbox/pyston/releases{/id}","deployments_url":"https://api.github.com/repos/dropbox/pyston/deployments","created_at":"2014-04-01T07:17:20Z","updated_at":"2016-09-30T21:45:32Z","pushed_at":"2016-09-30T15:11:47Z","git_url":"git://github.com/dropbox/pyston.git","ssh_url":"git@github.com:dropbox/pyston.git","clone_url":"https://github.com/dropbox/pyston.git","svn_url":"https://github.com/dropbox/pyston","homepage":"https://tech.dropbox.com/2014/04/introducing-pyston-an-upcoming-jit-based-python-implementation/","size":43336,"stargazers_count":4015,"watchers_count":4015,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":275,"mirror_url":null,"open_issues_count":110,"forks":275,"open_issues":110,"watchers":4015,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/dropbox/pyston/pulls/1377"},"html":{"href":"https://github.com/dropbox/pyston/pull/1377"},"issue":{"href":"https://api.github.com/repos/dropbox/pyston/issues/1377"},"comments":{"href":"https://api.github.com/repos/dropbox/pyston/issues/1377/comments"},"review_comments":{"href":"https://api.github.com/repos/dropbox/pyston/pulls/1377/comments"},"review_comment":{"href":"https://api.github.com/repos/dropbox/pyston/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/dropbox/pyston/pulls/1377/commits"},"statuses":{"href":"https://api.github.com/repos/dropbox/pyston/statuses/6f7d44fd48a1e124514e46250233678d83392cd4"}}}}
{"actor":{"display_login":"kmod"}}