{"action":"created","comment":{"url":"https://api.github.com/repos/nodejs/node/pulls/comments/54014130","id":54014130,"diff_hunk":"@@ -54,99 +128,132 @@ exports.active = function(item) {\n };\n \n function listOnTimeout() {\n-  var msecs = this.msecs;\n-  var list = this;\n+  var list = this._list;\n+  var msecs = list.msecs;\n \n   debug('timeout callback %d', msecs);\n \n-  var now = Timer.now();\n+  var now = TimerWrap.now();\n   debug('now: %s', now);\n \n-  var diff, first, threw;\n-  while (first = L.peek(list)) {\n-    diff = now - first._idleStart;\n+  var diff, timer, threw;\n+  while (timer = L.peek(list)) {\n+    diff = now - timer._idleStart;\n+\n+    // Check if this loop iteration is too early for the next timer.\n+    // This happens if there are more timers scheduled for later in the list.\n     if (diff < msecs) {\n-      list.start(msecs - diff, 0);\n+      this.start(msecs - diff, 0);\n       debug('%d list wait because diff is %d', msecs, diff);\n       return;\n-    } else {\n-      L.remove(first);\n-      assert(first !== L.peek(list));\n+    }\n+\n+    // The actual logic for when a timeout happens.\n \n-      if (!first._onTimeout) continue;\n+    L.remove(timer);\n+    assert(timer !== L.peek(list));\n \n-      // v0.4 compatibility: if the timer callback throws and the\n+    if (!timer._onTimeout) continue;\n+\n+    var domain = timer.domain;\n+    if (domain) {\n+\n+      // If the timer callback throws and the\n       // domain or uncaughtException handler ignore the exception,\n       // other timers that expire on this tick should still run.\n       //\n-      // https://github.com/joyent/node/issues/2631\n-      var domain = first.domain;\n-      if (domain && domain._disposed)\n+      // https://github.com/nodejs/node-v0.x-archive/issues/2631\n+      if (domain._disposed)\n         continue;\n \n-      try {\n-        if (domain)\n-          domain.enter();\n-        threw = true;\n-        first._called = true;\n-        first._onTimeout();\n-        if (domain)\n-          domain.exit();\n-        threw = false;\n-      } finally {\n-        if (threw) {\n-          // We need to continue processing after domain error handling\n-          // is complete, but not by using whatever domain was left over\n-          // when the timeout threw its exception.\n-          var oldDomain = process.domain;\n-          process.domain = null;\n-          process.nextTick(listOnTimeoutNT, list);\n-          process.domain = oldDomain;\n-        }\n-      }\n+      domain.enter();\n     }\n+\n+    tryOnTimeout(timer, list);\n+\n+    if (domain)\n+      domain.exit();\n   }\n \n+  // If `L.peek(list)` returned nothing, the list was either empty or we have\n+  // called all of the timer timeouts.\n+  // As such, we can remove the list and clean up the TimerWrap C++ handle.\n   debug('%d list empty', msecs);\n   assert(L.isEmpty(list));\n-  list.close();\n-  delete lists[msecs];\n+  this.close();\n+  if (list._unrefed === true) {\n+    delete unrefedLists[msecs];\n+  } else {\n+    delete refedLists[msecs];\n+  }\n+}\n+\n+\n+// An optimization so that the try/finally only de-optimizes what is in this\n+// smaller function.\n+function tryOnTimeout(timer, list) {\n+  timer._called = true;\n+  var threw = true;\n+  try {\n+    timer._onTimeout();\n+    threw = false;\n+  } finally {\n+    if (!threw) return;","path":"lib/timers.js","position":269,"original_position":269,"commit_id":"ab41a47f56efaea352dc7f90ea82acb4a072bb25","original_commit_id":"ab41a47f56efaea352dc7f90ea82acb4a072bb25","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":"@mscdex wait, I'm returning early here but there don't see to be any negative effects?","created_at":"2016-02-24T22:01:02Z","updated_at":"2016-02-24T22:01:02Z","html_url":"https://github.com/nodejs/node/pull/4007#discussion_r54014130","pull_request_url":"https://api.github.com/repos/nodejs/node/pulls/4007","_links":{"self":{"href":"https://api.github.com/repos/nodejs/node/pulls/comments/54014130"},"html":{"href":"https://github.com/nodejs/node/pull/4007#discussion_r54014130"},"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":"2016-02-24T22:01:02Z","closed_at":null,"merged_at":null,"merge_commit_sha":"eff112c09ad105f5c3f4612ab219fd036f78ee0a","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/ab41a47f56efaea352dc7f90ea82acb4a072bb25","head":{"label":"Fishrock123:refactor-timers","ref":"refactor-timers","sha":"ab41a47f56efaea352dc7f90ea82acb4a072bb25","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. Currently active major 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}","deployments_url":"https://api.github.com/repos/Fishrock123/node/deployments","created_at":"2015-01-22T15:15:17Z","updated_at":"2016-01-11T17:20:30Z","pushed_at":"2016-02-24T18:51:35Z","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":178107,"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":"81e35b5a2956604a6a3b9bb795a74a342702cc12","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}","deployments_url":"https://api.github.com/repos/nodejs/node/deployments","created_at":"2014-11-26T19:57:11Z","updated_at":"2016-02-24T20:44:18Z","pushed_at":"2016-02-24T21:39:41Z","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":214166,"stargazers_count":20482,"watchers_count":20482,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":2552,"mirror_url":null,"open_issues_count":755,"forks":2552,"open_issues":755,"watchers":20482,"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/ab41a47f56efaea352dc7f90ea82acb4a072bb25"}}}}