Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize BlacklistFactory #1295

Merged
merged 2 commits into from Nov 29, 2017

Conversation

@quartata
Copy link
Member

commented Nov 29, 2017

This changes two things:

  • replace the closure with a callable object
  • instead of using named groups, construct a mapping of regular group indices to line numbers. When I tried this previously, it didn't work out because any groups in the keywords themselves would throw off the count, but now I count the number of groups in each keyword when constructing the mapping.

This went from a 2x slowdown to a little above a 1.25x slowdown.

quartata added 2 commits Nov 29, 2017
@codecov-io

This comment has been minimized.

Copy link

commented Nov 29, 2017

Codecov Report

Merging #1295 into master will increase coverage by 0.26%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1295      +/-   ##
==========================================
+ Coverage   64.45%   64.72%   +0.26%     
==========================================
  Files           6        6              
  Lines        1840     1851      +11     
==========================================
+ Hits         1186     1198      +12     
+ Misses        654      653       -1
Impacted Files Coverage Δ
findspam.py 85.48% <100%> (+0.6%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2dac6f...79fc39d. Read the comment docs.

@Undo1

This comment has been minimized.

Copy link
Member

commented Nov 29, 2017

!!/approve

Approved with PullApprove

@Undo1 Undo1 merged commit 9633a5d into Charcoal-SE:master Nov 29, 2017
6 checks passed
6 checks passed
ci/circleci Your tests passed on CircleCI!
Details
code-review/pullapprove Approved by Undo1
Details
codeclimate All good!
Details
codecov/patch 100% of diff hit (target 64.45%)
Details
codecov/project 64.72% (+0.26%) compared to e2dac6f
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Undo1 added a commit that referenced this pull request Nov 29, 2017
Revert "Merge pull request #1295 from quartata/factory_optimize --autopull"

This reverts commit 9633a5d, reversing
changes made to 6f4b3b0.

Revert "Merge pull request #1276 from quartata/which_pattern"

This reverts commit a6087a2, reversing
changes made to fb1e32e.

Revert "Merge pull request #1275 from quartata/which_pattern"

This reverts commit fb1e32e, reversing
changes made to 6a8be29.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.