Skip to content

⚡ Improving performance.#2

Open
dravenk wants to merge 4 commits into
smallnest:masterfrom
dravenk:master
Open

⚡ Improving performance.#2
dravenk wants to merge 4 commits into
smallnest:masterfrom
dravenk:master

Conversation

@dravenk
Copy link
Copy Markdown

@dravenk dravenk commented Mar 18, 2019

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

@coveralls
Copy link
Copy Markdown

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
Copy Markdown
Author

dravenk commented Mar 18, 2019

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

Comment thread smooth_weighted.go
for i := 0; i < len(items); i++ {
w := items[i]

if w == nil {
Copy link
Copy Markdown
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.

Comment thread smooth_weighted.go

// Next returns next selected server.
func (w *SW) Next() interface{} {
i := w.nextWeighted()
Copy link
Copy Markdown
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.

Comment thread smooth_weighted.go
}

// nextWeighted returns next selected weighted object.
func (w *SW) nextWeighted() *smoothWeighted {
Copy link
Copy Markdown
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
Copy Markdown
Author

dravenk commented Mar 18, 2019

All tests are passed. Please reviews.

@dravenk
Copy link
Copy Markdown
Author

dravenk commented Mar 18, 2019

OK.Cover more tests.Please reviews.

Comment thread roundrobin_weighted.go Outdated
if w.cw == 0 {
return nil
}
// When does this happen?
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
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.

Comment thread smooth_weighted.go

// Next returns next selected server.
func (w *SW) Next() interface{} {
i := w.nextWeighted()
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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