Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29341426: Issue 235 - Speed up ElemHide.getSelectorsForDomain (Closed)

Created:
May 16, 2016, 8:27 p.m. by kzar
Modified:
May 18, 2016, 4:01 p.m.
Visibility:
Public.

Description

Issue 235 - Speed up ElemHide.getSelectorsForDomain

Patch Set 1 #

Total comments: 6

Patch Set 2 : Cache filtersByDomain[domain] #

Total comments: 8

Patch Set 3 : Addressed feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+72 lines, -12 lines) Patch
M lib/elemHide.js View 1 2 5 chunks +72 lines, -12 lines 0 comments Download

Messages

Total messages: 8
kzar
Patch Set 1
May 16, 2016, 8:29 p.m. (2016-05-16 20:29:39 UTC) #1
Sebastian Noack
https://codereview.adblockplus.org/29341426/diff/29341427/lib/elemHide.js File lib/elemHide.js (right): https://codereview.adblockplus.org/29341426/diff/29341427/lib/elemHide.js#newcode158 lib/elemHide.js:158: if (!(domain in filtersByDomain)) I think we should cache ...
May 17, 2016, 9:47 a.m. (2016-05-17 09:47:50 UTC) #2
kzar
Patch Set 2 : Cache filtersByDomain[domain] https://codereview.adblockplus.org/29341426/diff/29341427/lib/elemHide.js File lib/elemHide.js (right): https://codereview.adblockplus.org/29341426/diff/29341427/lib/elemHide.js#newcode158 lib/elemHide.js:158: if (!(domain in ...
May 17, 2016, 10:05 a.m. (2016-05-17 10:05:46 UTC) #3
Sebastian Noack
LGTM, assuming the logic works like described in your comments, i.e. like it did before. ...
May 17, 2016, 10:58 a.m. (2016-05-17 10:58:22 UTC) #4
Wladimir Palant
https://codereview.adblockplus.org/29341426/diff/29341462/lib/elemHide.js File lib/elemHide.js (right): https://codereview.adblockplus.org/29341426/diff/29341462/lib/elemHide.js#newcode154 lib/elemHide.js:154: let domainMatches = filter.domains || {"": true}; You are ...
May 18, 2016, 1:38 p.m. (2016-05-18 13:38:06 UTC) #5
Sebastian Noack
https://codereview.adblockplus.org/29341426/diff/29341462/lib/elemHide.js File lib/elemHide.js (right): https://codereview.adblockplus.org/29341426/diff/29341462/lib/elemHide.js#newcode154 lib/elemHide.js:154: let domainMatches = filter.domains || {"": true}; On 2016/05/18 ...
May 18, 2016, 1:48 p.m. (2016-05-18 13:48:26 UTC) #6
kzar
Patch Set 3 : Addressed feedback https://codereview.adblockplus.org/29341426/diff/29341462/lib/elemHide.js File lib/elemHide.js (right): https://codereview.adblockplus.org/29341426/diff/29341462/lib/elemHide.js#newcode154 lib/elemHide.js:154: let domainMatches = ...
May 18, 2016, 2:23 p.m. (2016-05-18 14:23:01 UTC) #7
Wladimir Palant
May 18, 2016, 3:38 p.m. (2016-05-18 15:38:50 UTC) #8
LGTM

Powered by Google App Engine
This is Rietveld