{"action":"created","comment":{"url":"https://api.github.com/repos/nodejs/node/pulls/comments/47797335","id":47797335,"diff_hunk":"@@ -10,36 +10,92 @@ const kOnTimeout = Timer.kOnTimeout | 0;\n // Timeout values > TIMEOUT_MAX are set to 1.\n const TIMEOUT_MAX = 2147483647; // 2^31-1\n \n-// IDLE TIMEOUTS\n+\n+// HOW and WHY the timers implementation works the way it does.\n+//\n+// Since many parts of Node.js and user applications rely on timeouts, there may\n+// be a very large amount of timeouts scheduled at any given time.\n+// Therefore, it is very important that the timers implementation is very fast\n+// and efficient.\n+//\n+// Note: It is suggested you first read though the lib/internal/linkedlist.js\n+// linked list implementation. How it works is somewhat counter-intuitive to\n+// most JavaScript code, and timers depend on it extensively.\n+//\n+// In order to be as efficient as possible, the implementation is does as little\n+// as possible, and only when required.\n+//\n+// Object maps are kept which contain linked lists keyed by their duration in\n+// milliseconds.\n+// The linked lists within are TimerWrap C++ handles with a linked list appended\n+// to their JavaScript representation.\n+//\n+// With this, constant-time insertion (append), removal, and timeout is possible\n+// in the JavaScript layer. Any one list of timers is able to be sorted by just\n+// appending to it because all timers share the same duration. Therefore, any\n+// timer added later will always have been scheduled to timeout later, thus\n+// only needing to be appended. Any less-than constant overhead is contained\n+// within the TimerWrap's inherently faster libuv bindings.\n+// There is, however, a minor overhead from accessing the specific list from the\n+// map, or creating a new one, but it is trivial in comparison.\n+// Removal from an object-property linked list is also constant-time as can be\n+// seen in the lib/internal/linkedlist.js implementation.\n+// Timeouts only need to process any timers due to currently timeout, which will\n+// always be at the  beginning of the list for reasons stated above. Any timers\n+// after the first one encountered that does not yet need to timeout will also\n+// always be due to timeout at a later time.\n //\n-// Because often many sockets will have the same idle timeout we will not\n-// use one timeout watcher per item. It is too much overhead.  Instead\n-// we'll use a single watcher for all sockets with the same timeout value\n-// and a linked list. This technique is described in the libev manual:\n+// The technique is described (in lesser detail) within the libev manual:\n // http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#Be_smart_about_timeouts\n \n-// Object containing all lists, timers\n-// key = time in milliseconds\n-// value = list\n-var lists = {};\n \n+// Object maps containing linked lists of timers, keyed and sorted by their\n+// duration in milliseconds.\n+// - key = time in milliseconds\n+// - value = linked list\n+const refedLists = {};\n+const unrefedLists = {};\n+// The difference between these two objects is that the former contains timers\n+// that will keep the process open if they are the only thing left, while the\n+// latter will not.\n \n-// call this whenever the item is active (not idle)\n-// it will reset its timeout.\n-// the main function - creates lists on demand and the watchers associated\n-// with them.\n+\n+// Schedule or re-schedule a timer.\n+// Call this whenever the item is active (not idle).\n+// The item must have been enroll()'d first.\n exports.active = function(item) {\n+  insert(item, false);\n+};\n+\n+// Internal APIs that need timeouts should use `_unrefActive()` instead of\n+// `active()` so that they do not unnecessarily keep the process open.\n+exports._unrefActive = function(item) {\n+  insert(item, true);\n+};\n+\n+\n+// The underlying logic for scheduling or re-scheduling a timer.\n+//\n+// Appends a timer onto the end of an existing timers list, or creates a new\n+// TimerWrap backed list if one does not already exist for the specified timeout\n+// duration.\n+function insert(item, unrefed) {\n   const msecs = item._idleTimeout;\n   if (msecs < 0 || msecs === undefined) return;\n \n   item._idleStart = Timer.now();\n \n-  var list;\n+  const lists = unrefed ? unrefedLists : refedLists;\n \n-  if (lists[msecs]) {\n-    list = lists[msecs];\n-  } else {\n+  // Use an existing list if there is one, otherwise we need to make a new one.\n+  var list = lists[msecs];\n+  if (!list) {\n+    // Make a new linked list of timers.\n+    // The list object is a TimerWrap, a C++ handle backing the timers for\n+    // efficiency, and the linked list is appended onto it.\n     list = new Timer();\n+    if (unrefed) list.unref();","path":"lib/timers.js","position":114,"original_position":114,"commit_id":"313ffc2e2cbf091bb55405650216fd6a926e748f","original_commit_id":"313ffc2e2cbf091bb55405650216fd6a926e748f","user":{"login":"bnoordhuis","id":275871,"avatar_url":"https://avatars.githubusercontent.com/u/275871?v=3","gravatar_id":"","url":"https://api.github.com/users/bnoordhuis","html_url":"https://github.com/bnoordhuis","followers_url":"https://api.github.com/users/bnoordhuis/followers","following_url":"https://api.github.com/users/bnoordhuis/following{/other_user}","gists_url":"https://api.github.com/users/bnoordhuis/gists{/gist_id}","starred_url":"https://api.github.com/users/bnoordhuis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bnoordhuis/subscriptions","organizations_url":"https://api.github.com/users/bnoordhuis/orgs","repos_url":"https://api.github.com/users/bnoordhuis/repos","events_url":"https://api.github.com/users/bnoordhuis/events{/privacy}","received_events_url":"https://api.github.com/users/bnoordhuis/received_events","type":"User","site_admin":false},"body":"Prefer strict equality here so the full-codegen can emit an immediate boolean check instead of a ToBoolean IC.","created_at":"2015-12-16T16:24:19Z","updated_at":"2015-12-16T16:24:19Z","html_url":"https://github.com/nodejs/node/pull/4007#discussion_r47797335","pull_request_url":"https://api.github.com/repos/nodejs/node/pulls/4007","_links":{"self":{"href":"https://api.github.com/repos/nodejs/node/pulls/comments/47797335"},"html":{"href":"https://github.com/nodejs/node/pull/4007#discussion_r47797335"},"pull_request":{"href":"https://api.github.com/repos/nodejs/node/pulls/4007"}}},"pull_request":{"url":"https://api.github.com/repos/nodejs/node/pulls/4007","id":51689097,"html_url":"https://github.com/nodejs/node/pull/4007","diff_url":"https://github.com/nodejs/node/pull/4007.diff","patch_url":"https://github.com/nodejs/node/pull/4007.patch","issue_url":"https://api.github.com/repos/nodejs/node/issues/4007","number":4007,"state":"open","locked":false,"title":"Refactor Timers","user":{"login":"Fishrock123","id":1093990,"avatar_url":"https://avatars.githubusercontent.com/u/1093990?v=3","gravatar_id":"","url":"https://api.github.com/users/Fishrock123","html_url":"https://github.com/Fishrock123","followers_url":"https://api.github.com/users/Fishrock123/followers","following_url":"https://api.github.com/users/Fishrock123/following{/other_user}","gists_url":"https://api.github.com/users/Fishrock123/gists{/gist_id}","starred_url":"https://api.github.com/users/Fishrock123/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Fishrock123/subscriptions","organizations_url":"https://api.github.com/users/Fishrock123/orgs","repos_url":"https://api.github.com/users/Fishrock123/repos","events_url":"https://api.github.com/users/Fishrock123/events{/privacy}","received_events_url":"https://api.github.com/users/Fishrock123/received_events","type":"User","site_admin":false},"body":"### Commit 1\r\n\r\nConsolidates the implementation of regular and internal (_unrefActive)\r\ntimers.\r\n\r\nIncludes an optimization for listOnTimeout() that previously only\r\ninternal timers had. (_runOnTimeout)\r\n\r\nAlso includes some minor other cleanup.\r\n\r\n### Commit 2\r\n\r\nDescribes the How and Why of the timers implementation, as well as\r\nadding comments in spots that should allow for an easier understanding\r\nof what is going on.\r\n\r\nThe timers implementation is very efficient, at a cost.\r\nThat cost is readable understandability, and this aims to improve that.\r\n\r\n-----\r\n\r\ncc @bnoordhuis / @piscisaureus / @trevnorris / @misterdjules / etc?\r\n\r\nThis attempts to improve the timers implementation by consolidating the internal and regular timer logic.\r\n\r\nI have not yet run performance testing / profiling on this, so feel free to help me out there if you are able to but I will hopefully be able to get to it shortly.\r\n\r\nTests pass locally, CI: https://ci.nodejs.org/job/node-test-commit/1237/\r\n\r\n--\r\n\r\ncc @nodejs/documentation and @nodejs/inclusivity I'd like thoughts on the format of the comments and their content and if they fit in well as code comments, as well as how understandable this is to people who are unfamiliar with the code.","created_at":"2015-11-24T17:15:03Z","updated_at":"2015-12-16T16:24:19Z","closed_at":null,"merged_at":null,"merge_commit_sha":"7358950bd6df79617ebc2283b29e403469fd0def","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/nodejs/node/pulls/4007/commits","review_comments_url":"https://api.github.com/repos/nodejs/node/pulls/4007/comments","review_comment_url":"https://api.github.com/repos/nodejs/node/pulls/comments{/number}","comments_url":"https://api.github.com/repos/nodejs/node/issues/4007/comments","statuses_url":"https://api.github.com/repos/nodejs/node/statuses/313ffc2e2cbf091bb55405650216fd6a926e748f","head":{"label":"Fishrock123:refactor-timers","ref":"refactor-timers","sha":"313ffc2e2cbf091bb55405650216fd6a926e748f","user":{"login":"Fishrock123","id":1093990,"avatar_url":"https://avatars.githubusercontent.com/u/1093990?v=3","gravatar_id":"","url":"https://api.github.com/users/Fishrock123","html_url":"https://github.com/Fishrock123","followers_url":"https://api.github.com/users/Fishrock123/followers","following_url":"https://api.github.com/users/Fishrock123/following{/other_user}","gists_url":"https://api.github.com/users/Fishrock123/gists{/gist_id}","starred_url":"https://api.github.com/users/Fishrock123/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Fishrock123/subscriptions","organizations_url":"https://api.github.com/users/Fishrock123/orgs","repos_url":"https://api.github.com/users/Fishrock123/repos","events_url":"https://api.github.com/users/Fishrock123/events{/privacy}","received_events_url":"https://api.github.com/users/Fishrock123/received_events","type":"User","site_admin":false},"repo":{"id":29684638,"name":"node","full_name":"Fishrock123/node","owner":{"login":"Fishrock123","id":1093990,"avatar_url":"https://avatars.githubusercontent.com/u/1093990?v=3","gravatar_id":"","url":"https://api.github.com/users/Fishrock123","html_url":"https://github.com/Fishrock123","followers_url":"https://api.github.com/users/Fishrock123/followers","following_url":"https://api.github.com/users/Fishrock123/following{/other_user}","gists_url":"https://api.github.com/users/Fishrock123/gists{/gist_id}","starred_url":"https://api.github.com/users/Fishrock123/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Fishrock123/subscriptions","organizations_url":"https://api.github.com/users/Fishrock123/orgs","repos_url":"https://api.github.com/users/Fishrock123/repos","events_url":"https://api.github.com/users/Fishrock123/events{/privacy}","received_events_url":"https://api.github.com/users/Fishrock123/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/Fishrock123/node","description":"My personal clone of Node. Stable branches are maintained.","fork":true,"url":"https://api.github.com/repos/Fishrock123/node","forks_url":"https://api.github.com/repos/Fishrock123/node/forks","keys_url":"https://api.github.com/repos/Fishrock123/node/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Fishrock123/node/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Fishrock123/node/teams","hooks_url":"https://api.github.com/repos/Fishrock123/node/hooks","issue_events_url":"https://api.github.com/repos/Fishrock123/node/issues/events{/number}","events_url":"https://api.github.com/repos/Fishrock123/node/events","assignees_url":"https://api.github.com/repos/Fishrock123/node/assignees{/user}","branches_url":"https://api.github.com/repos/Fishrock123/node/branches{/branch}","tags_url":"https://api.github.com/repos/Fishrock123/node/tags","blobs_url":"https://api.github.com/repos/Fishrock123/node/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Fishrock123/node/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Fishrock123/node/git/refs{/sha}","trees_url":"https://api.github.com/repos/Fishrock123/node/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Fishrock123/node/statuses/{sha}","languages_url":"https://api.github.com/repos/Fishrock123/node/languages","stargazers_url":"https://api.github.com/repos/Fishrock123/node/stargazers","contributors_url":"https://api.github.com/repos/Fishrock123/node/contributors","subscribers_url":"https://api.github.com/repos/Fishrock123/node/subscribers","subscription_url":"https://api.github.com/repos/Fishrock123/node/subscription","commits_url":"https://api.github.com/repos/Fishrock123/node/commits{/sha}","git_commits_url":"https://api.github.com/repos/Fishrock123/node/git/commits{/sha}","comments_url":"https://api.github.com/repos/Fishrock123/node/comments{/number}","issue_comment_url":"https://api.github.com/repos/Fishrock123/node/issues/comments{/number}","contents_url":"https://api.github.com/repos/Fishrock123/node/contents/{+path}","compare_url":"https://api.github.com/repos/Fishrock123/node/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Fishrock123/node/merges","archive_url":"https://api.github.com/repos/Fishrock123/node/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Fishrock123/node/downloads","issues_url":"https://api.github.com/repos/Fishrock123/node/issues{/number}","pulls_url":"https://api.github.com/repos/Fishrock123/node/pulls{/number}","milestones_url":"https://api.github.com/repos/Fishrock123/node/milestones{/number}","notifications_url":"https://api.github.com/repos/Fishrock123/node/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Fishrock123/node/labels{/name}","releases_url":"https://api.github.com/repos/Fishrock123/node/releases{/id}","created_at":"2015-01-22T15:15:17Z","updated_at":"2015-09-15T14:44:44Z","pushed_at":"2015-12-16T01:50:47Z","git_url":"git://github.com/Fishrock123/node.git","ssh_url":"git@github.com:Fishrock123/node.git","clone_url":"https://github.com/Fishrock123/node.git","svn_url":"https://github.com/Fishrock123/node","homepage":"https://nodejs.org","size":161008,"stargazers_count":1,"watchers_count":1,"language":"JavaScript","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":1,"default_branch":"master"}},"base":{"label":"nodejs:master","ref":"master","sha":"9538fd02e52d27f40d48bf0ebd65515dfd47cf9f","user":{"login":"nodejs","id":9950313,"avatar_url":"https://avatars.githubusercontent.com/u/9950313?v=3","gravatar_id":"","url":"https://api.github.com/users/nodejs","html_url":"https://github.com/nodejs","followers_url":"https://api.github.com/users/nodejs/followers","following_url":"https://api.github.com/users/nodejs/following{/other_user}","gists_url":"https://api.github.com/users/nodejs/gists{/gist_id}","starred_url":"https://api.github.com/users/nodejs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nodejs/subscriptions","organizations_url":"https://api.github.com/users/nodejs/orgs","repos_url":"https://api.github.com/users/nodejs/repos","events_url":"https://api.github.com/users/nodejs/events{/privacy}","received_events_url":"https://api.github.com/users/nodejs/received_events","type":"Organization","site_admin":false},"repo":{"id":27193779,"name":"node","full_name":"nodejs/node","owner":{"login":"nodejs","id":9950313,"avatar_url":"https://avatars.githubusercontent.com/u/9950313?v=3","gravatar_id":"","url":"https://api.github.com/users/nodejs","html_url":"https://github.com/nodejs","followers_url":"https://api.github.com/users/nodejs/followers","following_url":"https://api.github.com/users/nodejs/following{/other_user}","gists_url":"https://api.github.com/users/nodejs/gists{/gist_id}","starred_url":"https://api.github.com/users/nodejs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nodejs/subscriptions","organizations_url":"https://api.github.com/users/nodejs/orgs","repos_url":"https://api.github.com/users/nodejs/repos","events_url":"https://api.github.com/users/nodejs/events{/privacy}","received_events_url":"https://api.github.com/users/nodejs/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/nodejs/node","description":"Node.js JavaScript runtime :sparkles::turtle::rocket::sparkles:","fork":false,"url":"https://api.github.com/repos/nodejs/node","forks_url":"https://api.github.com/repos/nodejs/node/forks","keys_url":"https://api.github.com/repos/nodejs/node/keys{/key_id}","collaborators_url":"https://api.github.com/repos/nodejs/node/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/nodejs/node/teams","hooks_url":"https://api.github.com/repos/nodejs/node/hooks","issue_events_url":"https://api.github.com/repos/nodejs/node/issues/events{/number}","events_url":"https://api.github.com/repos/nodejs/node/events","assignees_url":"https://api.github.com/repos/nodejs/node/assignees{/user}","branches_url":"https://api.github.com/repos/nodejs/node/branches{/branch}","tags_url":"https://api.github.com/repos/nodejs/node/tags","blobs_url":"https://api.github.com/repos/nodejs/node/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/nodejs/node/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/nodejs/node/git/refs{/sha}","trees_url":"https://api.github.com/repos/nodejs/node/git/trees{/sha}","statuses_url":"https://api.github.com/repos/nodejs/node/statuses/{sha}","languages_url":"https://api.github.com/repos/nodejs/node/languages","stargazers_url":"https://api.github.com/repos/nodejs/node/stargazers","contributors_url":"https://api.github.com/repos/nodejs/node/contributors","subscribers_url":"https://api.github.com/repos/nodejs/node/subscribers","subscription_url":"https://api.github.com/repos/nodejs/node/subscription","commits_url":"https://api.github.com/repos/nodejs/node/commits{/sha}","git_commits_url":"https://api.github.com/repos/nodejs/node/git/commits{/sha}","comments_url":"https://api.github.com/repos/nodejs/node/comments{/number}","issue_comment_url":"https://api.github.com/repos/nodejs/node/issues/comments{/number}","contents_url":"https://api.github.com/repos/nodejs/node/contents/{+path}","compare_url":"https://api.github.com/repos/nodejs/node/compare/{base}...{head}","merges_url":"https://api.github.com/repos/nodejs/node/merges","archive_url":"https://api.github.com/repos/nodejs/node/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/nodejs/node/downloads","issues_url":"https://api.github.com/repos/nodejs/node/issues{/number}","pulls_url":"https://api.github.com/repos/nodejs/node/pulls{/number}","milestones_url":"https://api.github.com/repos/nodejs/node/milestones{/number}","notifications_url":"https://api.github.com/repos/nodejs/node/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/nodejs/node/labels{/name}","releases_url":"https://api.github.com/repos/nodejs/node/releases{/id}","created_at":"2014-11-26T19:57:11Z","updated_at":"2015-12-16T16:09:48Z","pushed_at":"2015-12-16T16:08:35Z","git_url":"git://github.com/nodejs/node.git","ssh_url":"git@github.com:nodejs/node.git","clone_url":"https://github.com/nodejs/node.git","svn_url":"https://github.com/nodejs/node","homepage":"https://nodejs.org","size":177011,"stargazers_count":18342,"watchers_count":18342,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":2060,"mirror_url":null,"open_issues_count":611,"forks":2060,"open_issues":611,"watchers":18342,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/nodejs/node/pulls/4007"},"html":{"href":"https://github.com/nodejs/node/pull/4007"},"issue":{"href":"https://api.github.com/repos/nodejs/node/issues/4007"},"comments":{"href":"https://api.github.com/repos/nodejs/node/issues/4007/comments"},"review_comments":{"href":"https://api.github.com/repos/nodejs/node/pulls/4007/comments"},"review_comment":{"href":"https://api.github.com/repos/nodejs/node/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/nodejs/node/pulls/4007/commits"},"statuses":{"href":"https://api.github.com/repos/nodejs/node/statuses/313ffc2e2cbf091bb55405650216fd6a926e748f"}}}}