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

⚡ Improving performance. #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

⚡ Improving performance. #2

wants to merge 4 commits into from

Conversation

dravenk
Copy link

@dravenk dravenk commented Mar 18, 2019

Improving of method gcb(). Adding tests for the method.Please reviews.

@coveralls
Copy link

coveralls commented Mar 18, 2019

Pull Request Test Coverage Report for Build 17

  • 11 of 11 (100.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+19.6%) to 94.964%

Files with Coverage Reduction New Missed Lines %
random_weighted.go 1 97.14%
Totals Coverage Status
Change from base Build 9: 19.6%
Covered Lines: 132
Relevant Lines: 139

💛 - Coveralls

@dravenk dravenk changed the title ⚡ Improving performance. Improving of method gcb :WIP: ⚡ Improving performance. Improving of method gcb Mar 18, 2019
@dravenk
Copy link
Author

dravenk commented Mar 18, 2019

I read the test coverage report and was confused as to why test coverage had declined.

@@ -92,10 +83,6 @@ func nextSmoothWeighted(items []*smoothWeighted) (best *smoothWeighted) {
for i := 0; i < len(items); i++ {
w := items[i]

if w == nil {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the parameters call into the method nextSmoothWeighted() always is greater than 1. Another way, whether it is len(items) =0; i !< 0; len(items) > 1 , items[i] != nil; So, those code never gets executed.

@@ -66,23 +66,14 @@ func (w *SW) All() map[interface{}]int {

// Next returns next selected server.
func (w *SW) Next() interface{} {
i := w.nextWeighted()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if w.n == 0; just return; No additional method calls are required.

}

// nextWeighted returns next selected weighted object.
func (w *SW) nextWeighted() *smoothWeighted {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be more concise to make these judgments in next.

@dravenk dravenk changed the title :WIP: ⚡ Improving performance. Improving of method gcb ⚡ Improving performance. Mar 18, 2019
@dravenk
Copy link
Author

dravenk commented Mar 18, 2019

All tests are passed. Please reviews.

@dravenk
Copy link
Author

dravenk commented Mar 18, 2019

OK.Cover more tests.Please reviews.

if w.cw == 0 {
return nil
}
// When does this happen?
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I has create a issue talk about those code. #3
but now, revert that code in this commit.

@@ -66,23 +66,14 @@ func (w *SW) All() map[interface{}]int {

// Next returns next selected server.
func (w *SW) Next() interface{} {
i := w.nextWeighted()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why removed this line?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because can be return nextSmoothWeighted(w.items).Item
look blow:

// Next returns next selected server.
func (w *SW) Next() interface{} {
	switch w.n {
	case 0:
		return nil
	case 1:
		return w.items[0].Item
	default:
		return nextSmoothWeighted(w.items).Item
	}
}

@dravenk
Copy link
Author

dravenk commented Mar 20, 2019

Please reviews!

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