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 Implementation #1

Merged
merged 10 commits into from
Jan 13, 2020

Conversation

achille-roussel
Copy link
Contributor

Hello,

This PR modifies the internal implementation to reduce the CPU and memory footprint of the algorithms, here are numbers to compare upstream with this change on the benchmark that I added:

benchmark                  old ns/op     new ns/op     delta
BenchmarkNaturalBreaks     292883        143310        -51.07%

benchmark                  old allocs     new allocs     delta
BenchmarkNaturalBreaks     355            4              -98.87%

benchmark                  old bytes     new bytes     delta
BenchmarkNaturalBreaks     49840         28784         -42.25%

@coveralls
Copy link

coveralls commented Aug 30, 2019

Pull Request Test Coverage Report for Build 21

  • 146 of 152 (96.05%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-2.3%) to 97.727%

Changes Missing Coverage Covered Lines Changed/Added Lines %
jenks.go 146 152 96.05%
Totals Coverage Status
Change from base Build 12: -2.3%
Covered Lines: 258
Relevant Lines: 264

💛 - Coveralls

@YetAnotherMatt YetAnotherMatt merged commit 3877a28 into ThinkingLogic:master Jan 13, 2020
@YetAnotherMatt
Copy link
Member

Hi @achille-roussel I can only apologise for ignoring your PR for so long (and I see you've understandably forged ahead with your own fork). I had notifications turned off, and have only just realised that this PR existed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants