{"action":"created","comment":{"url":"https://api.github.com/repos/nodejs/node/pulls/comments/45770340","id":45770340,"diff_hunk":"@@ -10,36 +10,84 @@ 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+// As such, it is very important that the timers implementation be very fast and\n+// efficent.\n+//\n+// Note: It is suggested you first read though the lib/internal/linkedlist.js\n+// linked list implementaion. How it works is somewhat counter-intuitive to\n+// most JavScript code, and timers depend on it extensively.\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+// To achieve a high level of efficiency, the implementation is as distributed\n+// and lazy as possible.\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, each list of timers with the same duration reduces the overhead of\n+// trying to track when all of the timers should timeout by only tracking the\n+// first timeout, and adjusting to sooner or later timeouts when applicable.\n+// This technique is descirbed in the libev manual:\n // http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#Be_smart_about_timeouts\n+//\n+// The timers are distributed into separate lists by milliseconds primarly to\n+// reduce the impact of the linked list's O(n) (linear) insertion time.\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 miliseconds.\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+// Inserts a timer into an existing timers list, or creates a new TimerWrap\n+// backed list if one does not already exist for the specified timeout 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+  // Use an existing list of there is one, otherwise we need to make a new one.\n   if (lists[msecs]) {\n     list = lists[msecs];\n   } else {\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();\n+    list._unrefed = unrefed;","path":"lib/timers.js","position":103,"original_position":103,"commit_id":"2e798fa0bd9d1122edcda19f4471da2ed45fe2ff","original_commit_id":"2e798fa0bd9d1122edcda19f4471da2ed45fe2ff","user":{"login":"alexlamsl","id":1898413,"avatar_url":"https://avatars.githubusercontent.com/u/1898413?v=3","gravatar_id":"","url":"https://api.github.com/users/alexlamsl","html_url":"https://github.com/alexlamsl","followers_url":"https://api.github.com/users/alexlamsl/followers","following_url":"https://api.github.com/users/alexlamsl/following{/other_user}","gists_url":"https://api.github.com/users/alexlamsl/gists{/gist_id}","starred_url":"https://api.github.com/users/alexlamsl/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexlamsl/subscriptions","organizations_url":"https://api.github.com/users/alexlamsl/orgs","repos_url":"https://api.github.com/users/alexlamsl/repos","events_url":"https://api.github.com/users/alexlamsl/events{/privacy}","received_events_url":"https://api.github.com/users/alexlamsl/received_events","type":"User","site_admin":false},"body":"Could just store a reference to `lists` so just have one-line `delete` instead of branch down there?","created_at":"2015-11-24T17:54:15Z","updated_at":"2015-11-24T17:54:15Z","html_url":"https://github.com/nodejs/node/pull/4007#discussion_r45770340","pull_request_url":"https://api.github.com/repos/nodejs/node/pulls/4007","_links":{"self":{"href":"https://api.github.com/repos/nodejs/node/pulls/comments/45770340"},"html":{"href":"https://github.com/nodejs/node/pull/4007#discussion_r45770340"},"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-11-24T17:54:15Z","closed_at":null,"merged_at":null,"merge_commit_sha":"e09c29ef36eba910ba06c121560a4b7c1cacc172","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/2e798fa0bd9d1122edcda19f4471da2ed45fe2ff","head":{"label":"Fishrock123:refactor-timers","ref":"refactor-timers","sha":"2e798fa0bd9d1122edcda19f4471da2ed45fe2ff","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-11-24T16:08:34Z","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":151583,"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":"41519fd1a4041e46c42d94c303fcb13fa2fc9344","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-11-24T17:36:56Z","pushed_at":"2015-11-24T17:44:18Z","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":167864,"stargazers_count":17615,"watchers_count":17615,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1843,"mirror_url":null,"open_issues_count":594,"forks":1843,"open_issues":594,"watchers":17615,"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/2e798fa0bd9d1122edcda19f4471da2ed45fe2ff"}}}}