{"action":"created","comment":{"url":"https://api.github.com/repos/juttle/bubo/pulls/comments/47174405","id":47174405,"diff_hunk":"@@ -88,11 +92,13 @@ bool AttributesTable::prepare_entry_buffer(const v8::Local<v8::Object>& pt,\n     for (u_int32_t i = 0; i < length; ++i) {\n         v8::Local<v8::Value> key = Nan::Get(keys, i).ToLocalChecked();\n         v8::Local<v8::String> key_str(key->ToString());\n-        if (!ignored_attributes_->IsEmpty() && Nan::HasOwnProperty(Nan::New(*ignored_attributes_), key_str).FromJust()) {\n+        v8::String::Utf8Value tag(key);\n+        std::string tag_str(*tag);\n+\n+        if (!ignored_attributes_.empty() && _contains(ignored_attributes_, tag_str)) {","path":"src/attrs-table.cc","position":28,"original_position":28,"commit_id":"83638245bddb4b0dd3d44a58ad9c0b357e3681a1","original_commit_id":"83638245bddb4b0dd3d44a58ad9c0b357e3681a1","user":{"login":"demmer","id":3308504,"avatar_url":"https://avatars.githubusercontent.com/u/3308504?v=3","gravatar_id":"","url":"https://api.github.com/users/demmer","html_url":"https://github.com/demmer","followers_url":"https://api.github.com/users/demmer/followers","following_url":"https://api.github.com/users/demmer/following{/other_user}","gists_url":"https://api.github.com/users/demmer/gists{/gist_id}","starred_url":"https://api.github.com/users/demmer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/demmer/subscriptions","organizations_url":"https://api.github.com/users/demmer/orgs","repos_url":"https://api.github.com/users/demmer/repos","events_url":"https://api.github.com/users/demmer/events{/privacy}","received_events_url":"https://api.github.com/users/demmer/received_events","type":"User","site_admin":false},"body":"Oh wait - scratch that. In this new approach you're actually copying the ignored_attributes vector instead of sharing it via a pointer.\r\n\r\nSo in fact, you should change `ignored_attributes` to be a `std::vector<std::string>*` and then make sure it's non-null before calling `_contains`.","created_at":"2015-12-10T00:26:15Z","updated_at":"2015-12-10T00:26:15Z","html_url":"https://github.com/juttle/bubo/pull/3#discussion_r47174405","pull_request_url":"https://api.github.com/repos/juttle/bubo/pulls/3","_links":{"self":{"href":"https://api.github.com/repos/juttle/bubo/pulls/comments/47174405"},"html":{"href":"https://github.com/juttle/bubo/pull/3#discussion_r47174405"},"pull_request":{"href":"https://api.github.com/repos/juttle/bubo/pulls/3"}}},"pull_request":{"url":"https://api.github.com/repos/juttle/bubo/pulls/3","id":53197026,"html_url":"https://github.com/juttle/bubo/pull/3","diff_url":"https://github.com/juttle/bubo/pull/3.diff","patch_url":"https://github.com/juttle/bubo/pull/3.patch","issue_url":"https://api.github.com/repos/juttle/bubo/issues/3","number":3,"state":"open","locked":false,"title":"Make ignored attributes an array","user":{"login":"davidvgalbraith","id":2757844,"avatar_url":"https://avatars.githubusercontent.com/u/2757844?v=3","gravatar_id":"","url":"https://api.github.com/users/davidvgalbraith","html_url":"https://github.com/davidvgalbraith","followers_url":"https://api.github.com/users/davidvgalbraith/followers","following_url":"https://api.github.com/users/davidvgalbraith/following{/other_user}","gists_url":"https://api.github.com/users/davidvgalbraith/gists{/gist_id}","starred_url":"https://api.github.com/users/davidvgalbraith/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidvgalbraith/subscriptions","organizations_url":"https://api.github.com/users/davidvgalbraith/orgs","repos_url":"https://api.github.com/users/davidvgalbraith/repos","events_url":"https://api.github.com/users/davidvgalbraith/events{/privacy}","received_events_url":"https://api.github.com/users/davidvgalbraith/received_events","type":"User","site_admin":false},"body":"Currently you initialize Bubo's ignored attributes as an object whose keys are the attributes you want to ignore. This is weird. It should just be an array of the attributes you want to ignore. And then the `ignored_attributes` all over the Bubo code are `std::vector`s instead of v8 objects. This didn't noticeably change performance.\r\n\r\n@demmer ","created_at":"2015-12-09T23:56:41Z","updated_at":"2015-12-10T00:26:15Z","closed_at":null,"merged_at":null,"merge_commit_sha":"ef1ecaa370bd3fbd6223dd2629ff17712f953531","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/juttle/bubo/pulls/3/commits","review_comments_url":"https://api.github.com/repos/juttle/bubo/pulls/3/comments","review_comment_url":"https://api.github.com/repos/juttle/bubo/pulls/comments{/number}","comments_url":"https://api.github.com/repos/juttle/bubo/issues/3/comments","statuses_url":"https://api.github.com/repos/juttle/bubo/statuses/83638245bddb4b0dd3d44a58ad9c0b357e3681a1","head":{"label":"juttle:array-ignored","ref":"array-ignored","sha":"83638245bddb4b0dd3d44a58ad9c0b357e3681a1","user":{"login":"juttle","id":8988633,"avatar_url":"https://avatars.githubusercontent.com/u/8988633?v=3","gravatar_id":"","url":"https://api.github.com/users/juttle","html_url":"https://github.com/juttle","followers_url":"https://api.github.com/users/juttle/followers","following_url":"https://api.github.com/users/juttle/following{/other_user}","gists_url":"https://api.github.com/users/juttle/gists{/gist_id}","starred_url":"https://api.github.com/users/juttle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/juttle/subscriptions","organizations_url":"https://api.github.com/users/juttle/orgs","repos_url":"https://api.github.com/users/juttle/repos","events_url":"https://api.github.com/users/juttle/events{/privacy}","received_events_url":"https://api.github.com/users/juttle/received_events","type":"Organization","site_admin":false},"repo":{"id":46840215,"name":"bubo","full_name":"juttle/bubo","owner":{"login":"juttle","id":8988633,"avatar_url":"https://avatars.githubusercontent.com/u/8988633?v=3","gravatar_id":"","url":"https://api.github.com/users/juttle","html_url":"https://github.com/juttle","followers_url":"https://api.github.com/users/juttle/followers","following_url":"https://api.github.com/users/juttle/following{/other_user}","gists_url":"https://api.github.com/users/juttle/gists{/gist_id}","starred_url":"https://api.github.com/users/juttle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/juttle/subscriptions","organizations_url":"https://api.github.com/users/juttle/orgs","repos_url":"https://api.github.com/users/juttle/repos","events_url":"https://api.github.com/users/juttle/events{/privacy}","received_events_url":"https://api.github.com/users/juttle/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/juttle/bubo","description":"memory efficient node.js object cache","fork":false,"url":"https://api.github.com/repos/juttle/bubo","forks_url":"https://api.github.com/repos/juttle/bubo/forks","keys_url":"https://api.github.com/repos/juttle/bubo/keys{/key_id}","collaborators_url":"https://api.github.com/repos/juttle/bubo/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/juttle/bubo/teams","hooks_url":"https://api.github.com/repos/juttle/bubo/hooks","issue_events_url":"https://api.github.com/repos/juttle/bubo/issues/events{/number}","events_url":"https://api.github.com/repos/juttle/bubo/events","assignees_url":"https://api.github.com/repos/juttle/bubo/assignees{/user}","branches_url":"https://api.github.com/repos/juttle/bubo/branches{/branch}","tags_url":"https://api.github.com/repos/juttle/bubo/tags","blobs_url":"https://api.github.com/repos/juttle/bubo/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/juttle/bubo/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/juttle/bubo/git/refs{/sha}","trees_url":"https://api.github.com/repos/juttle/bubo/git/trees{/sha}","statuses_url":"https://api.github.com/repos/juttle/bubo/statuses/{sha}","languages_url":"https://api.github.com/repos/juttle/bubo/languages","stargazers_url":"https://api.github.com/repos/juttle/bubo/stargazers","contributors_url":"https://api.github.com/repos/juttle/bubo/contributors","subscribers_url":"https://api.github.com/repos/juttle/bubo/subscribers","subscription_url":"https://api.github.com/repos/juttle/bubo/subscription","commits_url":"https://api.github.com/repos/juttle/bubo/commits{/sha}","git_commits_url":"https://api.github.com/repos/juttle/bubo/git/commits{/sha}","comments_url":"https://api.github.com/repos/juttle/bubo/comments{/number}","issue_comment_url":"https://api.github.com/repos/juttle/bubo/issues/comments{/number}","contents_url":"https://api.github.com/repos/juttle/bubo/contents/{+path}","compare_url":"https://api.github.com/repos/juttle/bubo/compare/{base}...{head}","merges_url":"https://api.github.com/repos/juttle/bubo/merges","archive_url":"https://api.github.com/repos/juttle/bubo/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/juttle/bubo/downloads","issues_url":"https://api.github.com/repos/juttle/bubo/issues{/number}","pulls_url":"https://api.github.com/repos/juttle/bubo/pulls{/number}","milestones_url":"https://api.github.com/repos/juttle/bubo/milestones{/number}","notifications_url":"https://api.github.com/repos/juttle/bubo/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/juttle/bubo/labels{/name}","releases_url":"https://api.github.com/repos/juttle/bubo/releases{/id}","created_at":"2015-11-25T05:44:05Z","updated_at":"2015-11-25T05:48:22Z","pushed_at":"2015-12-09T23:56:41Z","git_url":"git://github.com/juttle/bubo.git","ssh_url":"git@github.com:juttle/bubo.git","clone_url":"https://github.com/juttle/bubo.git","svn_url":"https://github.com/juttle/bubo","homepage":null,"size":39,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":1,"forks":0,"open_issues":1,"watchers":0,"default_branch":"master"}},"base":{"label":"juttle:master","ref":"master","sha":"7407621a023b9a044ee2e2938a674e077f57926e","user":{"login":"juttle","id":8988633,"avatar_url":"https://avatars.githubusercontent.com/u/8988633?v=3","gravatar_id":"","url":"https://api.github.com/users/juttle","html_url":"https://github.com/juttle","followers_url":"https://api.github.com/users/juttle/followers","following_url":"https://api.github.com/users/juttle/following{/other_user}","gists_url":"https://api.github.com/users/juttle/gists{/gist_id}","starred_url":"https://api.github.com/users/juttle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/juttle/subscriptions","organizations_url":"https://api.github.com/users/juttle/orgs","repos_url":"https://api.github.com/users/juttle/repos","events_url":"https://api.github.com/users/juttle/events{/privacy}","received_events_url":"https://api.github.com/users/juttle/received_events","type":"Organization","site_admin":false},"repo":{"id":46840215,"name":"bubo","full_name":"juttle/bubo","owner":{"login":"juttle","id":8988633,"avatar_url":"https://avatars.githubusercontent.com/u/8988633?v=3","gravatar_id":"","url":"https://api.github.com/users/juttle","html_url":"https://github.com/juttle","followers_url":"https://api.github.com/users/juttle/followers","following_url":"https://api.github.com/users/juttle/following{/other_user}","gists_url":"https://api.github.com/users/juttle/gists{/gist_id}","starred_url":"https://api.github.com/users/juttle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/juttle/subscriptions","organizations_url":"https://api.github.com/users/juttle/orgs","repos_url":"https://api.github.com/users/juttle/repos","events_url":"https://api.github.com/users/juttle/events{/privacy}","received_events_url":"https://api.github.com/users/juttle/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/juttle/bubo","description":"memory efficient node.js object cache","fork":false,"url":"https://api.github.com/repos/juttle/bubo","forks_url":"https://api.github.com/repos/juttle/bubo/forks","keys_url":"https://api.github.com/repos/juttle/bubo/keys{/key_id}","collaborators_url":"https://api.github.com/repos/juttle/bubo/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/juttle/bubo/teams","hooks_url":"https://api.github.com/repos/juttle/bubo/hooks","issue_events_url":"https://api.github.com/repos/juttle/bubo/issues/events{/number}","events_url":"https://api.github.com/repos/juttle/bubo/events","assignees_url":"https://api.github.com/repos/juttle/bubo/assignees{/user}","branches_url":"https://api.github.com/repos/juttle/bubo/branches{/branch}","tags_url":"https://api.github.com/repos/juttle/bubo/tags","blobs_url":"https://api.github.com/repos/juttle/bubo/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/juttle/bubo/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/juttle/bubo/git/refs{/sha}","trees_url":"https://api.github.com/repos/juttle/bubo/git/trees{/sha}","statuses_url":"https://api.github.com/repos/juttle/bubo/statuses/{sha}","languages_url":"https://api.github.com/repos/juttle/bubo/languages","stargazers_url":"https://api.github.com/repos/juttle/bubo/stargazers","contributors_url":"https://api.github.com/repos/juttle/bubo/contributors","subscribers_url":"https://api.github.com/repos/juttle/bubo/subscribers","subscription_url":"https://api.github.com/repos/juttle/bubo/subscription","commits_url":"https://api.github.com/repos/juttle/bubo/commits{/sha}","git_commits_url":"https://api.github.com/repos/juttle/bubo/git/commits{/sha}","comments_url":"https://api.github.com/repos/juttle/bubo/comments{/number}","issue_comment_url":"https://api.github.com/repos/juttle/bubo/issues/comments{/number}","contents_url":"https://api.github.com/repos/juttle/bubo/contents/{+path}","compare_url":"https://api.github.com/repos/juttle/bubo/compare/{base}...{head}","merges_url":"https://api.github.com/repos/juttle/bubo/merges","archive_url":"https://api.github.com/repos/juttle/bubo/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/juttle/bubo/downloads","issues_url":"https://api.github.com/repos/juttle/bubo/issues{/number}","pulls_url":"https://api.github.com/repos/juttle/bubo/pulls{/number}","milestones_url":"https://api.github.com/repos/juttle/bubo/milestones{/number}","notifications_url":"https://api.github.com/repos/juttle/bubo/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/juttle/bubo/labels{/name}","releases_url":"https://api.github.com/repos/juttle/bubo/releases{/id}","created_at":"2015-11-25T05:44:05Z","updated_at":"2015-11-25T05:48:22Z","pushed_at":"2015-12-09T23:56:41Z","git_url":"git://github.com/juttle/bubo.git","ssh_url":"git@github.com:juttle/bubo.git","clone_url":"https://github.com/juttle/bubo.git","svn_url":"https://github.com/juttle/bubo","homepage":null,"size":39,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":1,"forks":0,"open_issues":1,"watchers":0,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/juttle/bubo/pulls/3"},"html":{"href":"https://github.com/juttle/bubo/pull/3"},"issue":{"href":"https://api.github.com/repos/juttle/bubo/issues/3"},"comments":{"href":"https://api.github.com/repos/juttle/bubo/issues/3/comments"},"review_comments":{"href":"https://api.github.com/repos/juttle/bubo/pulls/3/comments"},"review_comment":{"href":"https://api.github.com/repos/juttle/bubo/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/juttle/bubo/pulls/3/commits"},"statuses":{"href":"https://api.github.com/repos/juttle/bubo/statuses/83638245bddb4b0dd3d44a58ad9c0b357e3681a1"}}}}