{"action":"created","comment":{"url":"https://api.github.com/repos/m3db/m3db/pulls/comments/86197852","pull_request_review_id":6867722,"id":86197852,"diff_hunk":"@@ -574,99 +651,67 @@ func (s *session) Write(namespace, id string, t time.Time, value float64, unit x\n \t\treturn timestampErr\n \t}\n \n-\ts.RLock()\n-\tif s.state != stateOpen {\n+\tif s.RLock(); s.state != stateOpen {\n \t\ts.RUnlock()\n \t\treturn errSessionStateNotOpen\n \t}\n \n-\tmajority = atomic.LoadInt32(&s.majority)\n+\tc := s.writeStatePool.Get().(*writeState)\n+\tc.incref()\n \n-\tw := s.writeOpPool.Get()\n-\tw.namespace = ts.StringID(namespace)\n-\tw.request.ID = tsID.Data()\n-\tw.request.Datapoint.Value = value\n-\tw.request.Datapoint.Timestamp = timestamp\n-\tw.request.Datapoint.TimestampType = timeType\n-\tw.request.Datapoint.Annotation = annotation\n-\tw.completionFn = func(result interface{}, err error) {\n-\t\tvar snapshotSuccess int32\n-\t\tif err != nil {\n-\t\t\tatomic.AddInt32(&resultErrs, 1)\n-\t\t\tresultErrLock.Lock()\n-\t\t\terrors = append(errors, err)\n-\t\t\tresultErrLock.Unlock()\n-\t\t} else {\n-\t\t\tsnapshotSuccess = atomic.AddInt32(&success, 1)\n-\t\t}\n-\t\tremaining := atomic.AddInt32(&pending, -1)\n-\t\tdoneAll := remaining == 0\n-\t\tswitch s.writeLevel {\n-\t\tcase topology.ConsistencyLevelOne:\n-\t\t\tcomplete := snapshotSuccess > 0 || doneAll\n-\t\t\tif complete && atomic.CompareAndSwapInt32(&wgIsDone, 0, 1) {\n-\t\t\t\twg.Done()\n-\t\t\t}\n-\t\tcase topology.ConsistencyLevelMajority:\n-\t\t\tcomplete := snapshotSuccess >= majority || doneAll\n-\t\t\tif complete && atomic.CompareAndSwapInt32(&wgIsDone, 0, 1) {\n-\t\t\t\twg.Done()\n-\t\t\t}\n-\t\tcase topology.ConsistencyLevelAll:\n-\t\t\tif doneAll && atomic.CompareAndSwapInt32(&wgIsDone, 0, 1) {\n-\t\t\t\twg.Done()\n-\t\t\t}\n-\t\t}\n-\t\t// Finalize resources that can be returned\n-\t\tif doneAll {\n-\t\t\t// Return write to pool\n-\t\t\ts.writeOpPool.Put(w)\n-\t\t}\n-\t}\n+\tc.op, c.majority = s.writeOpPool.Get(), majority\n \n-\trouteErr := s.topoMap.RouteForEach(tsID, func(idx int, host topology.Host) {\n+\tc.op.namespace = ts.StringID(namespace)\n+\tc.op.request.ID = tsID.Data()\n+\tc.op.request.Datapoint.Value = value\n+\tc.op.request.Datapoint.Timestamp = timestamp\n+\tc.op.request.Datapoint.TimestampType = timeType\n+\tc.op.request.Datapoint.Annotation = annotation\n+\tc.op.completionFn = c.completionFn\n+\n+\tif err := s.topoMap.RouteForEach(tsID, func(idx int, host topology.Host) {\n \t\t// First count all the pending write requests to ensure\n \t\t// we count the amount we're going to be waiting for\n \t\t// before we enqueue the completion fns that rely on having\n \t\t// an accurate number of the pending requests when they execute\n-\t\tpending++\n-\t})\n-\tif routeErr == nil {\n-\t\t// Now enqueue the write requests\n-\t\trouteErr = s.topoMap.RouteForEach(tsID, func(idx int, host topology.Host) {\n-\t\t\tenqueued++\n-\t\t\tif err := s.queues[idx].Enqueue(w); err != nil && enqueueErr == nil {\n-\t\t\t\t// NB(r): if this ever happens we have a code bug, once we\n-\t\t\t\t// are in the read lock the current queues we are using should\n-\t\t\t\t// never be closed\n-\t\t\t\tenqueueErr = err\n-\t\t\t}\n-\t\t})\n+\t\tc.pending++\n+\t\tc.queues = append(c.queues, s.queues[idx])\n+\t}); err != nil {\n+\t\tc.decref()\n+\t\ts.RUnlock()\n+\t\treturn err\n \t}\n-\ts.RUnlock()\n \n-\tif routeErr != nil {\n-\t\treturn routeErr\n-\t}\n+\tc.Lock()\n \n-\tif enqueueErr != nil {\n-\t\ts.opts.InstrumentOptions().Logger().Errorf(\"failed to enqueue write: %v\", enqueueErr)\n-\t\treturn enqueueErr\n+\tfor i := range c.queues {\n+\t\tif err := c.queues[i].Enqueue(c.op); err != nil {\n+\t\t\tc.Unlock()\n+\t\t\tc.decref()\n+\n+\t\t\t// NB(r): if this ever happens we have a code bug, once we\n+\t\t\t// are in the read lock the current queues we are using should\n+\t\t\t// never be closed\n+\t\t\ts.RUnlock()\n+\t\t\ts.opts.InstrumentOptions().Logger().Errorf(\"failed to enqueue write: %v\", err)\n+\t\t\treturn err\n+\t\t}\n+\n+\t\tc.incref()\n+\t\tenqueued++\n \t}\n \n \t// Wait for writes to complete\n-\twg.Wait()\n+\ts.RUnlock()\n+\tc.condition.Wait()","path":"client/session.go","position":247,"original_position":247,"commit_id":"c4e212fb0ef967aa05905eaae35f81023d79126c","original_commit_id":"8188a23d57ebd7d3d1a496c57c547cd7c2b3c0db","user":{"login":"kobolog","id":318866,"avatar_url":"https://avatars.githubusercontent.com/u/318866?v=3","gravatar_id":"","url":"https://api.github.com/users/kobolog","html_url":"https://github.com/kobolog","followers_url":"https://api.github.com/users/kobolog/followers","following_url":"https://api.github.com/users/kobolog/following{/other_user}","gists_url":"https://api.github.com/users/kobolog/gists{/gist_id}","starred_url":"https://api.github.com/users/kobolog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kobolog/subscriptions","organizations_url":"https://api.github.com/users/kobolog/orgs","repos_url":"https://api.github.com/users/kobolog/repos","events_url":"https://api.github.com/users/kobolog/events{/privacy}","received_events_url":"https://api.github.com/users/kobolog/received_events","type":"User","site_admin":false},"body":"Spurious wake ups don't happen because Golang's condvars has nothing to do with pthreads' condvars. According to Golang authors, `It's built on Semacquire, which does not permit spurious interrupts`.","created_at":"2016-11-02T17:28:40Z","updated_at":"2016-11-02T17:28:40Z","html_url":"https://github.com/m3db/m3db/pull/156#discussion_r86197852","pull_request_url":"https://api.github.com/repos/m3db/m3db/pulls/156","_links":{"self":{"href":"https://api.github.com/repos/m3db/m3db/pulls/comments/86197852"},"html":{"href":"https://github.com/m3db/m3db/pull/156#discussion_r86197852"},"pull_request":{"href":"https://api.github.com/repos/m3db/m3db/pulls/156"}}},"pull_request":{"url":"https://api.github.com/repos/m3db/m3db/pulls/156","id":91872454,"html_url":"https://github.com/m3db/m3db/pull/156","diff_url":"https://github.com/m3db/m3db/pull/156.diff","patch_url":"https://github.com/m3db/m3db/pull/156.patch","issue_url":"https://api.github.com/repos/m3db/m3db/issues/156","number":156,"state":"open","locked":false,"title":"Reduce heap escapes in session.Write().","user":{"login":"kobolog","id":318866,"avatar_url":"https://avatars.githubusercontent.com/u/318866?v=3","gravatar_id":"","url":"https://api.github.com/users/kobolog","html_url":"https://github.com/kobolog","followers_url":"https://api.github.com/users/kobolog/followers","following_url":"https://api.github.com/users/kobolog/following{/other_user}","gists_url":"https://api.github.com/users/kobolog/gists{/gist_id}","starred_url":"https://api.github.com/users/kobolog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kobolog/subscriptions","organizations_url":"https://api.github.com/users/kobolog/orgs","repos_url":"https://api.github.com/users/kobolog/repos","events_url":"https://api.github.com/users/kobolog/events{/privacy}","received_events_url":"https://api.github.com/users/kobolog/received_events","type":"User","site_admin":false},"body":"Client-side runtime and heap profiles demonstrated that almost 50% of all allocations in m3dbingester happen in `session.Write()` due to inferior quality of Go's escape analysis algorithms. Essentially, the algorithm marks any variable that is captured in a lambda as heap-allocated (even if the lambda does not outlive the function scope). In our case, some lambdas in `session.Write()` might outlive the function scope, so almost all local variables are marked as escaping.\r\n\r\nThis PR explicitly recaptures all escaping variables in a pooled heap-allocated object, effectively reducing the number of heap allocations, live objects and the amount of time required to do a GC scan pass.\r\n\r\nHere's a list of TODOs:\r\n\r\n- [ ] Pooling of `ts.ID`.\r\n- [ ] Get rid of the last `RouteForEach()` lambda.\r\n- [x] Get rid of the `queues` slice operations.","created_at":"2016-11-01T22:00:13Z","updated_at":"2016-11-02T17:28:40Z","closed_at":null,"merged_at":null,"merge_commit_sha":"a4e870c8d2ac5cff4c12133bb99c8f5777aa5043","assignee":{"login":"robskillington","id":339615,"avatar_url":"https://avatars.githubusercontent.com/u/339615?v=3","gravatar_id":"","url":"https://api.github.com/users/robskillington","html_url":"https://github.com/robskillington","followers_url":"https://api.github.com/users/robskillington/followers","following_url":"https://api.github.com/users/robskillington/following{/other_user}","gists_url":"https://api.github.com/users/robskillington/gists{/gist_id}","starred_url":"https://api.github.com/users/robskillington/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/robskillington/subscriptions","organizations_url":"https://api.github.com/users/robskillington/orgs","repos_url":"https://api.github.com/users/robskillington/repos","events_url":"https://api.github.com/users/robskillington/events{/privacy}","received_events_url":"https://api.github.com/users/robskillington/received_events","type":"User","site_admin":false},"assignees":[{"login":"robskillington","id":339615,"avatar_url":"https://avatars.githubusercontent.com/u/339615?v=3","gravatar_id":"","url":"https://api.github.com/users/robskillington","html_url":"https://github.com/robskillington","followers_url":"https://api.github.com/users/robskillington/followers","following_url":"https://api.github.com/users/robskillington/following{/other_user}","gists_url":"https://api.github.com/users/robskillington/gists{/gist_id}","starred_url":"https://api.github.com/users/robskillington/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/robskillington/subscriptions","organizations_url":"https://api.github.com/users/robskillington/orgs","repos_url":"https://api.github.com/users/robskillington/repos","events_url":"https://api.github.com/users/robskillington/events{/privacy}","received_events_url":"https://api.github.com/users/robskillington/received_events","type":"User","site_admin":false}],"milestone":null,"commits_url":"https://api.github.com/repos/m3db/m3db/pulls/156/commits","review_comments_url":"https://api.github.com/repos/m3db/m3db/pulls/156/comments","review_comment_url":"https://api.github.com/repos/m3db/m3db/pulls/comments{/number}","comments_url":"https://api.github.com/repos/m3db/m3db/issues/156/comments","statuses_url":"https://api.github.com/repos/m3db/m3db/statuses/c4e212fb0ef967aa05905eaae35f81023d79126c","head":{"label":"m3db:kobolog/better-session-write","ref":"kobolog/better-session-write","sha":"c4e212fb0ef967aa05905eaae35f81023d79126c","user":{"login":"m3db","id":19937171,"avatar_url":"https://avatars.githubusercontent.com/u/19937171?v=3","gravatar_id":"","url":"https://api.github.com/users/m3db","html_url":"https://github.com/m3db","followers_url":"https://api.github.com/users/m3db/followers","following_url":"https://api.github.com/users/m3db/following{/other_user}","gists_url":"https://api.github.com/users/m3db/gists{/gist_id}","starred_url":"https://api.github.com/users/m3db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/m3db/subscriptions","organizations_url":"https://api.github.com/users/m3db/orgs","repos_url":"https://api.github.com/users/m3db/repos","events_url":"https://api.github.com/users/m3db/events{/privacy}","received_events_url":"https://api.github.com/users/m3db/received_events","type":"Organization","site_admin":false},"repo":{"id":61153677,"name":"m3db","full_name":"m3db/m3db","owner":{"login":"m3db","id":19937171,"avatar_url":"https://avatars.githubusercontent.com/u/19937171?v=3","gravatar_id":"","url":"https://api.github.com/users/m3db","html_url":"https://github.com/m3db","followers_url":"https://api.github.com/users/m3db/followers","following_url":"https://api.github.com/users/m3db/following{/other_user}","gists_url":"https://api.github.com/users/m3db/gists{/gist_id}","starred_url":"https://api.github.com/users/m3db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/m3db/subscriptions","organizations_url":"https://api.github.com/users/m3db/orgs","repos_url":"https://api.github.com/users/m3db/repos","events_url":"https://api.github.com/users/m3db/events{/privacy}","received_events_url":"https://api.github.com/users/m3db/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/m3db/m3db","description":"LSM time series database using M3TSZ compression ","fork":false,"url":"https://api.github.com/repos/m3db/m3db","forks_url":"https://api.github.com/repos/m3db/m3db/forks","keys_url":"https://api.github.com/repos/m3db/m3db/keys{/key_id}","collaborators_url":"https://api.github.com/repos/m3db/m3db/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/m3db/m3db/teams","hooks_url":"https://api.github.com/repos/m3db/m3db/hooks","issue_events_url":"https://api.github.com/repos/m3db/m3db/issues/events{/number}","events_url":"https://api.github.com/repos/m3db/m3db/events","assignees_url":"https://api.github.com/repos/m3db/m3db/assignees{/user}","branches_url":"https://api.github.com/repos/m3db/m3db/branches{/branch}","tags_url":"https://api.github.com/repos/m3db/m3db/tags","blobs_url":"https://api.github.com/repos/m3db/m3db/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/m3db/m3db/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/m3db/m3db/git/refs{/sha}","trees_url":"https://api.github.com/repos/m3db/m3db/git/trees{/sha}","statuses_url":"https://api.github.com/repos/m3db/m3db/statuses/{sha}","languages_url":"https://api.github.com/repos/m3db/m3db/languages","stargazers_url":"https://api.github.com/repos/m3db/m3db/stargazers","contributors_url":"https://api.github.com/repos/m3db/m3db/contributors","subscribers_url":"https://api.github.com/repos/m3db/m3db/subscribers","subscription_url":"https://api.github.com/repos/m3db/m3db/subscription","commits_url":"https://api.github.com/repos/m3db/m3db/commits{/sha}","git_commits_url":"https://api.github.com/repos/m3db/m3db/git/commits{/sha}","comments_url":"https://api.github.com/repos/m3db/m3db/comments{/number}","issue_comment_url":"https://api.github.com/repos/m3db/m3db/issues/comments{/number}","contents_url":"https://api.github.com/repos/m3db/m3db/contents/{+path}","compare_url":"https://api.github.com/repos/m3db/m3db/compare/{base}...{head}","merges_url":"https://api.github.com/repos/m3db/m3db/merges","archive_url":"https://api.github.com/repos/m3db/m3db/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/m3db/m3db/downloads","issues_url":"https://api.github.com/repos/m3db/m3db/issues{/number}","pulls_url":"https://api.github.com/repos/m3db/m3db/pulls{/number}","milestones_url":"https://api.github.com/repos/m3db/m3db/milestones{/number}","notifications_url":"https://api.github.com/repos/m3db/m3db/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/m3db/m3db/labels{/name}","releases_url":"https://api.github.com/repos/m3db/m3db/releases{/id}","deployments_url":"https://api.github.com/repos/m3db/m3db/deployments","created_at":"2016-06-14T20:29:45Z","updated_at":"2016-10-31T21:31:43Z","pushed_at":"2016-11-02T14:50:56Z","git_url":"git://github.com/m3db/m3db.git","ssh_url":"git@github.com:m3db/m3db.git","clone_url":"https://github.com/m3db/m3db.git","svn_url":"https://github.com/m3db/m3db","homepage":"http://uber.github.io/","size":3685,"stargazers_count":19,"watchers_count":19,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":9,"forks":0,"open_issues":9,"watchers":19,"default_branch":"master"}},"base":{"label":"m3db:master","ref":"master","sha":"77a11918c252c4890f535f6cf8c7ee1a17497a3e","user":{"login":"m3db","id":19937171,"avatar_url":"https://avatars.githubusercontent.com/u/19937171?v=3","gravatar_id":"","url":"https://api.github.com/users/m3db","html_url":"https://github.com/m3db","followers_url":"https://api.github.com/users/m3db/followers","following_url":"https://api.github.com/users/m3db/following{/other_user}","gists_url":"https://api.github.com/users/m3db/gists{/gist_id}","starred_url":"https://api.github.com/users/m3db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/m3db/subscriptions","organizations_url":"https://api.github.com/users/m3db/orgs","repos_url":"https://api.github.com/users/m3db/repos","events_url":"https://api.github.com/users/m3db/events{/privacy}","received_events_url":"https://api.github.com/users/m3db/received_events","type":"Organization","site_admin":false},"repo":{"id":61153677,"name":"m3db","full_name":"m3db/m3db","owner":{"login":"m3db","id":19937171,"avatar_url":"https://avatars.githubusercontent.com/u/19937171?v=3","gravatar_id":"","url":"https://api.github.com/users/m3db","html_url":"https://github.com/m3db","followers_url":"https://api.github.com/users/m3db/followers","following_url":"https://api.github.com/users/m3db/following{/other_user}","gists_url":"https://api.github.com/users/m3db/gists{/gist_id}","starred_url":"https://api.github.com/users/m3db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/m3db/subscriptions","organizations_url":"https://api.github.com/users/m3db/orgs","repos_url":"https://api.github.com/users/m3db/repos","events_url":"https://api.github.com/users/m3db/events{/privacy}","received_events_url":"https://api.github.com/users/m3db/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/m3db/m3db","description":"LSM time series database using M3TSZ compression ","fork":false,"url":"https://api.github.com/repos/m3db/m3db","forks_url":"https://api.github.com/repos/m3db/m3db/forks","keys_url":"https://api.github.com/repos/m3db/m3db/keys{/key_id}","collaborators_url":"https://api.github.com/repos/m3db/m3db/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/m3db/m3db/teams","hooks_url":"https://api.github.com/repos/m3db/m3db/hooks","issue_events_url":"https://api.github.com/repos/m3db/m3db/issues/events{/number}","events_url":"https://api.github.com/repos/m3db/m3db/events","assignees_url":"https://api.github.com/repos/m3db/m3db/assignees{/user}","branches_url":"https://api.github.com/repos/m3db/m3db/branches{/branch}","tags_url":"https://api.github.com/repos/m3db/m3db/tags","blobs_url":"https://api.github.com/repos/m3db/m3db/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/m3db/m3db/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/m3db/m3db/git/refs{/sha}","trees_url":"https://api.github.com/repos/m3db/m3db/git/trees{/sha}","statuses_url":"https://api.github.com/repos/m3db/m3db/statuses/{sha}","languages_url":"https://api.github.com/repos/m3db/m3db/languages","stargazers_url":"https://api.github.com/repos/m3db/m3db/stargazers","contributors_url":"https://api.github.com/repos/m3db/m3db/contributors","subscribers_url":"https://api.github.com/repos/m3db/m3db/subscribers","subscription_url":"https://api.github.com/repos/m3db/m3db/subscription","commits_url":"https://api.github.com/repos/m3db/m3db/commits{/sha}","git_commits_url":"https://api.github.com/repos/m3db/m3db/git/commits{/sha}","comments_url":"https://api.github.com/repos/m3db/m3db/comments{/number}","issue_comment_url":"https://api.github.com/repos/m3db/m3db/issues/comments{/number}","contents_url":"https://api.github.com/repos/m3db/m3db/contents/{+path}","compare_url":"https://api.github.com/repos/m3db/m3db/compare/{base}...{head}","merges_url":"https://api.github.com/repos/m3db/m3db/merges","archive_url":"https://api.github.com/repos/m3db/m3db/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/m3db/m3db/downloads","issues_url":"https://api.github.com/repos/m3db/m3db/issues{/number}","pulls_url":"https://api.github.com/repos/m3db/m3db/pulls{/number}","milestones_url":"https://api.github.com/repos/m3db/m3db/milestones{/number}","notifications_url":"https://api.github.com/repos/m3db/m3db/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/m3db/m3db/labels{/name}","releases_url":"https://api.github.com/repos/m3db/m3db/releases{/id}","deployments_url":"https://api.github.com/repos/m3db/m3db/deployments","created_at":"2016-06-14T20:29:45Z","updated_at":"2016-10-31T21:31:43Z","pushed_at":"2016-11-02T14:50:56Z","git_url":"git://github.com/m3db/m3db.git","ssh_url":"git@github.com:m3db/m3db.git","clone_url":"https://github.com/m3db/m3db.git","svn_url":"https://github.com/m3db/m3db","homepage":"http://uber.github.io/","size":3685,"stargazers_count":19,"watchers_count":19,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":9,"forks":0,"open_issues":9,"watchers":19,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/m3db/m3db/pulls/156"},"html":{"href":"https://github.com/m3db/m3db/pull/156"},"issue":{"href":"https://api.github.com/repos/m3db/m3db/issues/156"},"comments":{"href":"https://api.github.com/repos/m3db/m3db/issues/156/comments"},"review_comments":{"href":"https://api.github.com/repos/m3db/m3db/pulls/156/comments"},"review_comment":{"href":"https://api.github.com/repos/m3db/m3db/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/m3db/m3db/pulls/156/commits"},"statuses":{"href":"https://api.github.com/repos/m3db/m3db/statuses/c4e212fb0ef967aa05905eaae35f81023d79126c"}}}}
{"actor":{"display_login":"kobolog"}}