{"action":"created","comment":{"url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments/82649511","pull_request_review_id":3542719,"id":82649511,"diff_hunk":"@@ -0,0 +1,168 @@\n+/*-------------------------------------------------------------------------\n+ *\n+ * memaccounting_private.h\n+ *\t  This file contains declarations for memory accounting functions that\n+ *\t  are only supposed to be used by privileged callers such a memory managers.\n+ *\t  Other files should not include this file.\n+ *\n+ * Copyright (c) 2016, Pivotal Inc.\n+ *\n+ *-------------------------------------------------------------------------\n+ */\n+#ifndef MEMACCOUNTING_PRIVATE_H\n+#define MEMACCOUNTING_PRIVATE_H\n+\n+#include \"utils/memaccounting.h\"\n+\n+extern MemoryAccountIdType liveAccountStartId;\n+extern MemoryAccountIdType nextAccountId;\n+\n+/* MemoryAccount is the fundamental data structure to record memory usage */\n+typedef struct MemoryAccount {\n+\tNodeTag type;\n+\tMemoryOwnerType ownerType;\n+\n+\tuint64 allocated;\n+\tuint64 freed;\n+\tuint64 peak;\n+\t/*\n+\t * Maximum targeted allocation for an owner. Peak usage can be tracked to\n+\t * check if the allocation is overshooting\n+\t */\n+\tuint64 maxLimit;\n+\n+\tMemoryAccountIdType id;\n+\tMemoryAccountIdType parentId;\n+} MemoryAccount;\n+\n+typedef struct MemoryAccountArray{\n+\tMemoryAccountIdType accountCount;\n+\tMemoryAccountIdType arraySize;\n+\t// array of pointers to memory accounts of size accountCount\n+\tMemoryAccount** allAccounts;\n+} MemoryAccountArray;\n+\n+extern MemoryAccountArray* shortLivingMemoryAccountArray;\n+extern MemoryAccount* longLivingMemoryAccountArray[MEMORY_OWNER_TYPE_END_LONG_LIVING + 1];\n+\n+extern MemoryAccount *SharedChunkHeadersMemoryAccount;\n+\n+extern uint64 MemoryAccountingOutstandingBalance;\n+extern uint64 MemoryAccountingPeakBalance;\n+\n+static inline bool\n+MemoryAccounting_IsValidAccount(MemoryAccountIdType id)\n+{\n+\tAssertImply(NULL == shortLivingMemoryAccountArray, liveAccountStartId == nextAccountId);\n+\treturn ((id >= liveAccountStartId &&","path":"src/include/utils/memaccounting_private.h","position":57,"original_position":57,"commit_id":"77b73232ea18f2c588dfccf95ede9201e4526cf1","original_commit_id":"1c861f9753119b142c0945daf5228d38b6cfba99","user":{"login":"foyzur","id":1253020,"avatar_url":"https://avatars.githubusercontent.com/u/1253020?v=3","gravatar_id":"","url":"https://api.github.com/users/foyzur","html_url":"https://github.com/foyzur","followers_url":"https://api.github.com/users/foyzur/followers","following_url":"https://api.github.com/users/foyzur/following{/other_user}","gists_url":"https://api.github.com/users/foyzur/gists{/gist_id}","starred_url":"https://api.github.com/users/foyzur/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/foyzur/subscriptions","organizations_url":"https://api.github.com/users/foyzur/orgs","repos_url":"https://api.github.com/users/foyzur/repos","events_url":"https://api.github.com/users/foyzur/events{/privacy}","received_events_url":"https://api.github.com/users/foyzur/received_events","type":"User","site_admin":false},"body":"I used only one bool (the short-living) and kept the long living as is. This is for short-circuit performance optimization. If we compute both var, in many cases we will just not need the long living. Not sure if compiler is smart enough.","created_at":"2016-10-10T17:38:45Z","updated_at":"2016-10-10T17:38:45Z","html_url":"https://github.com/greenplum-db/gpdb/pull/1175#discussion_r82649511","pull_request_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1175","_links":{"self":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments/82649511"},"html":{"href":"https://github.com/greenplum-db/gpdb/pull/1175#discussion_r82649511"},"pull_request":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1175"}}},"pull_request":{"url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1175","id":87950581,"html_url":"https://github.com/greenplum-db/gpdb/pull/1175","diff_url":"https://github.com/greenplum-db/gpdb/pull/1175.diff","patch_url":"https://github.com/greenplum-db/gpdb/pull/1175.patch","issue_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/1175","number":1175,"state":"open","locked":false,"title":"Index based memory accounting","user":{"login":"foyzur","id":1253020,"avatar_url":"https://avatars.githubusercontent.com/u/1253020?v=3","gravatar_id":"","url":"https://api.github.com/users/foyzur","html_url":"https://github.com/foyzur","followers_url":"https://api.github.com/users/foyzur/followers","following_url":"https://api.github.com/users/foyzur/following{/other_user}","gists_url":"https://api.github.com/users/foyzur/gists{/gist_id}","starred_url":"https://api.github.com/users/foyzur/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/foyzur/subscriptions","organizations_url":"https://api.github.com/users/foyzur/orgs","repos_url":"https://api.github.com/users/foyzur/repos","events_url":"https://api.github.com/users/foyzur/events{/privacy}","received_events_url":"https://api.github.com/users/foyzur/received_events","type":"User","site_admin":false},"body":"Previously accounting tree and reference to accounts were pointer based. This prevents us from freeing accounts at the end of each statement as later statements of that transaction can potentially access memory and the corresponding accounts. This resulted in a perceived memory leak as the accounts were not freed per-statement.\r\n\r\nIn this implementation we convert pointers to index (or soft-pointers) that can refer to an array of accounts. There are two array of accounts: long-living account array (aka shared accounts across all the statements that we don't create once per-statement) and short living accounts array that contain one account per-operator. An unified index can refer to both long and short living accounts. We reserve the first set of index values for\r\nlong living account array and later values for short-living account array.\r\n\r\nAs we create short-living accounts, we allocate a monotonically increasing \"serial\" to these accounts similar to \"ID\". This ID can then be converted to a valid array index in the short-living account array by subtracting the lowest ID of the list of live short-living accounts. Using this \"lowest\" short-living ID we can also logically designate which past accounts are now dead and their memory is no longer accessible.\r\n\r\nWe save this logical ever increasing ID in all the places where we used to save account pointers. As this is a 64 bit counter, we don't perceive an overflow. But, we plan to implement an overflow support. We provide APIs to transform this logical ID to actual pointers by projecting this logical ID into actual array index (subtracting the start ID of currently live short-living accounts) and accessing the account at that array position.","created_at":"2016-10-04T18:43:11Z","updated_at":"2016-10-10T17:38:45Z","closed_at":null,"merged_at":null,"merge_commit_sha":"1615952ef3189c0d46fe18aa59cb0396bfa56c22","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1175/commits","review_comments_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1175/comments","review_comment_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments{/number}","comments_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/1175/comments","statuses_url":"https://api.github.com/repos/greenplum-db/gpdb/statuses/77b73232ea18f2c588dfccf95ede9201e4526cf1","head":{"label":"foyzur:indexed_accounts","ref":"indexed_accounts","sha":"77b73232ea18f2c588dfccf95ede9201e4526cf1","user":{"login":"foyzur","id":1253020,"avatar_url":"https://avatars.githubusercontent.com/u/1253020?v=3","gravatar_id":"","url":"https://api.github.com/users/foyzur","html_url":"https://github.com/foyzur","followers_url":"https://api.github.com/users/foyzur/followers","following_url":"https://api.github.com/users/foyzur/following{/other_user}","gists_url":"https://api.github.com/users/foyzur/gists{/gist_id}","starred_url":"https://api.github.com/users/foyzur/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/foyzur/subscriptions","organizations_url":"https://api.github.com/users/foyzur/orgs","repos_url":"https://api.github.com/users/foyzur/repos","events_url":"https://api.github.com/users/foyzur/events{/privacy}","received_events_url":"https://api.github.com/users/foyzur/received_events","type":"User","site_admin":false},"repo":{"id":46012445,"name":"gpdb","full_name":"foyzur/gpdb","owner":{"login":"foyzur","id":1253020,"avatar_url":"https://avatars.githubusercontent.com/u/1253020?v=3","gravatar_id":"","url":"https://api.github.com/users/foyzur","html_url":"https://github.com/foyzur","followers_url":"https://api.github.com/users/foyzur/followers","following_url":"https://api.github.com/users/foyzur/following{/other_user}","gists_url":"https://api.github.com/users/foyzur/gists{/gist_id}","starred_url":"https://api.github.com/users/foyzur/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/foyzur/subscriptions","organizations_url":"https://api.github.com/users/foyzur/orgs","repos_url":"https://api.github.com/users/foyzur/repos","events_url":"https://api.github.com/users/foyzur/events{/privacy}","received_events_url":"https://api.github.com/users/foyzur/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/foyzur/gpdb","description":"Pivotal Greenplum Database","fork":true,"url":"https://api.github.com/repos/foyzur/gpdb","forks_url":"https://api.github.com/repos/foyzur/gpdb/forks","keys_url":"https://api.github.com/repos/foyzur/gpdb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/foyzur/gpdb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/foyzur/gpdb/teams","hooks_url":"https://api.github.com/repos/foyzur/gpdb/hooks","issue_events_url":"https://api.github.com/repos/foyzur/gpdb/issues/events{/number}","events_url":"https://api.github.com/repos/foyzur/gpdb/events","assignees_url":"https://api.github.com/repos/foyzur/gpdb/assignees{/user}","branches_url":"https://api.github.com/repos/foyzur/gpdb/branches{/branch}","tags_url":"https://api.github.com/repos/foyzur/gpdb/tags","blobs_url":"https://api.github.com/repos/foyzur/gpdb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/foyzur/gpdb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/foyzur/gpdb/git/refs{/sha}","trees_url":"https://api.github.com/repos/foyzur/gpdb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/foyzur/gpdb/statuses/{sha}","languages_url":"https://api.github.com/repos/foyzur/gpdb/languages","stargazers_url":"https://api.github.com/repos/foyzur/gpdb/stargazers","contributors_url":"https://api.github.com/repos/foyzur/gpdb/contributors","subscribers_url":"https://api.github.com/repos/foyzur/gpdb/subscribers","subscription_url":"https://api.github.com/repos/foyzur/gpdb/subscription","commits_url":"https://api.github.com/repos/foyzur/gpdb/commits{/sha}","git_commits_url":"https://api.github.com/repos/foyzur/gpdb/git/commits{/sha}","comments_url":"https://api.github.com/repos/foyzur/gpdb/comments{/number}","issue_comment_url":"https://api.github.com/repos/foyzur/gpdb/issues/comments{/number}","contents_url":"https://api.github.com/repos/foyzur/gpdb/contents/{+path}","compare_url":"https://api.github.com/repos/foyzur/gpdb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/foyzur/gpdb/merges","archive_url":"https://api.github.com/repos/foyzur/gpdb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/foyzur/gpdb/downloads","issues_url":"https://api.github.com/repos/foyzur/gpdb/issues{/number}","pulls_url":"https://api.github.com/repos/foyzur/gpdb/pulls{/number}","milestones_url":"https://api.github.com/repos/foyzur/gpdb/milestones{/number}","notifications_url":"https://api.github.com/repos/foyzur/gpdb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/foyzur/gpdb/labels{/name}","releases_url":"https://api.github.com/repos/foyzur/gpdb/releases{/id}","deployments_url":"https://api.github.com/repos/foyzur/gpdb/deployments","created_at":"2015-11-11T21:59:37Z","updated_at":"2016-04-08T17:48:01Z","pushed_at":"2016-10-07T18:24:25Z","git_url":"git://github.com/foyzur/gpdb.git","ssh_url":"git@github.com:foyzur/gpdb.git","clone_url":"https://github.com/foyzur/gpdb.git","svn_url":"https://github.com/foyzur/gpdb","homepage":"http://greenplum.org","size":178099,"stargazers_count":0,"watchers_count":0,"language":"PLpgSQL","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":"greenplum-db:master","ref":"master","sha":"6580341ab62242f72738bfa865a690fcda4b5dee","user":{"login":"greenplum-db","id":14097842,"avatar_url":"https://avatars.githubusercontent.com/u/14097842?v=3","gravatar_id":"","url":"https://api.github.com/users/greenplum-db","html_url":"https://github.com/greenplum-db","followers_url":"https://api.github.com/users/greenplum-db/followers","following_url":"https://api.github.com/users/greenplum-db/following{/other_user}","gists_url":"https://api.github.com/users/greenplum-db/gists{/gist_id}","starred_url":"https://api.github.com/users/greenplum-db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/greenplum-db/subscriptions","organizations_url":"https://api.github.com/users/greenplum-db/orgs","repos_url":"https://api.github.com/users/greenplum-db/repos","events_url":"https://api.github.com/users/greenplum-db/events{/privacy}","received_events_url":"https://api.github.com/users/greenplum-db/received_events","type":"Organization","site_admin":false},"repo":{"id":44781140,"name":"gpdb","full_name":"greenplum-db/gpdb","owner":{"login":"greenplum-db","id":14097842,"avatar_url":"https://avatars.githubusercontent.com/u/14097842?v=3","gravatar_id":"","url":"https://api.github.com/users/greenplum-db","html_url":"https://github.com/greenplum-db","followers_url":"https://api.github.com/users/greenplum-db/followers","following_url":"https://api.github.com/users/greenplum-db/following{/other_user}","gists_url":"https://api.github.com/users/greenplum-db/gists{/gist_id}","starred_url":"https://api.github.com/users/greenplum-db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/greenplum-db/subscriptions","organizations_url":"https://api.github.com/users/greenplum-db/orgs","repos_url":"https://api.github.com/users/greenplum-db/repos","events_url":"https://api.github.com/users/greenplum-db/events{/privacy}","received_events_url":"https://api.github.com/users/greenplum-db/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/greenplum-db/gpdb","description":"Greenplum Database","fork":false,"url":"https://api.github.com/repos/greenplum-db/gpdb","forks_url":"https://api.github.com/repos/greenplum-db/gpdb/forks","keys_url":"https://api.github.com/repos/greenplum-db/gpdb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/greenplum-db/gpdb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/greenplum-db/gpdb/teams","hooks_url":"https://api.github.com/repos/greenplum-db/gpdb/hooks","issue_events_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/events{/number}","events_url":"https://api.github.com/repos/greenplum-db/gpdb/events","assignees_url":"https://api.github.com/repos/greenplum-db/gpdb/assignees{/user}","branches_url":"https://api.github.com/repos/greenplum-db/gpdb/branches{/branch}","tags_url":"https://api.github.com/repos/greenplum-db/gpdb/tags","blobs_url":"https://api.github.com/repos/greenplum-db/gpdb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/greenplum-db/gpdb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/greenplum-db/gpdb/git/refs{/sha}","trees_url":"https://api.github.com/repos/greenplum-db/gpdb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/greenplum-db/gpdb/statuses/{sha}","languages_url":"https://api.github.com/repos/greenplum-db/gpdb/languages","stargazers_url":"https://api.github.com/repos/greenplum-db/gpdb/stargazers","contributors_url":"https://api.github.com/repos/greenplum-db/gpdb/contributors","subscribers_url":"https://api.github.com/repos/greenplum-db/gpdb/subscribers","subscription_url":"https://api.github.com/repos/greenplum-db/gpdb/subscription","commits_url":"https://api.github.com/repos/greenplum-db/gpdb/commits{/sha}","git_commits_url":"https://api.github.com/repos/greenplum-db/gpdb/git/commits{/sha}","comments_url":"https://api.github.com/repos/greenplum-db/gpdb/comments{/number}","issue_comment_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/comments{/number}","contents_url":"https://api.github.com/repos/greenplum-db/gpdb/contents/{+path}","compare_url":"https://api.github.com/repos/greenplum-db/gpdb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/greenplum-db/gpdb/merges","archive_url":"https://api.github.com/repos/greenplum-db/gpdb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/greenplum-db/gpdb/downloads","issues_url":"https://api.github.com/repos/greenplum-db/gpdb/issues{/number}","pulls_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls{/number}","milestones_url":"https://api.github.com/repos/greenplum-db/gpdb/milestones{/number}","notifications_url":"https://api.github.com/repos/greenplum-db/gpdb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/greenplum-db/gpdb/labels{/name}","releases_url":"https://api.github.com/repos/greenplum-db/gpdb/releases{/id}","deployments_url":"https://api.github.com/repos/greenplum-db/gpdb/deployments","created_at":"2015-10-23T00:25:17Z","updated_at":"2016-10-10T03:00:04Z","pushed_at":"2016-10-10T06:56:18Z","git_url":"git://github.com/greenplum-db/gpdb.git","ssh_url":"git@github.com:greenplum-db/gpdb.git","clone_url":"https://github.com/greenplum-db/gpdb.git","svn_url":"https://github.com/greenplum-db/gpdb","homepage":"http://greenplum.org","size":229896,"stargazers_count":1626,"watchers_count":1626,"language":"PLpgSQL","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":457,"mirror_url":null,"open_issues_count":117,"forks":457,"open_issues":117,"watchers":1626,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1175"},"html":{"href":"https://github.com/greenplum-db/gpdb/pull/1175"},"issue":{"href":"https://api.github.com/repos/greenplum-db/gpdb/issues/1175"},"comments":{"href":"https://api.github.com/repos/greenplum-db/gpdb/issues/1175/comments"},"review_comments":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1175/comments"},"review_comment":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1175/commits"},"statuses":{"href":"https://api.github.com/repos/greenplum-db/gpdb/statuses/77b73232ea18f2c588dfccf95ede9201e4526cf1"}}}}
{"actor":{"display_login":"foyzur"}}