Skip to content

[percentile] change GKArray methods to value receivers#379

Merged
jeerim merged 1 commit intomasterfrom
jeerim/percentile_non_pointer
Jul 12, 2017
Merged

[percentile] change GKArray methods to value receivers#379
jeerim merged 1 commit intomasterfrom
jeerim/percentile_non_pointer

Conversation

@jeerim
Copy link
Copy Markdown
Contributor

@jeerim jeerim commented Jul 11, 2017

What does this PR do?

Changes the methods of GKArray to value receivers.

Motivation

In order to make the sketches more efficient by reducing GC overhead.

Additional Notes

Anything else we should know when reviewing?

@jeerim jeerim requested a review from olivielpeau July 11, 2017 16:50
Copy link
Copy Markdown
Member

@olivielpeau olivielpeau left a comment

Choose a reason for hiding this comment

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

👍 if you're sure this has a positive impact on the performance

There are a few commented-out lines to remove

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you remove this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you remove this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let's remove this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ack, sorry - I should have checked more carefully.

@jeerim jeerim force-pushed the jeerim/percentile_non_pointer branch from feb08e6 to b8fd6a3 Compare July 12, 2017 13:21
@jeerim jeerim merged commit 787e47c into master Jul 12, 2017
@jeerim jeerim deleted the jeerim/percentile_non_pointer branch July 12, 2017 14:03
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.

2 participants