Hash#each_key instead of Hash#keys.each. Faster code. #3017

Merged
merged 1 commit into from Oct 20, 2014

Conversation

Projects
None yet
5 participants
@mattr-

This comment has been minimized.

Show comment
Hide comment
@mattr-

mattr- Oct 20, 2014

Member

Numbers or it's not really faster. 😃

Member

mattr- commented Oct 20, 2014

Numbers or it's not really faster. 😃

@glaucocustodio

This comment has been minimized.

Show comment
Hide comment
@glaucocustodio

glaucocustodio Oct 20, 2014

Contributor

You can check out the numbers at link embedded, it's ~ 20% faster.

Contributor

glaucocustodio commented Oct 20, 2014

You can check out the numbers at link embedded, it's ~ 20% faster.

@ixti

This comment has been minimized.

Show comment
Hide comment
@ixti

ixti Oct 20, 2014

Member

Hash#keys creates new Array and then you call #each on that array, while #each_key iterates over Hash keys without creating new objects.

Member

ixti commented Oct 20, 2014

Hash#keys creates new Array and then you call #each on that array, while #each_key iterates over Hash keys without creating new objects.

@ixti

This comment has been minimized.

Show comment
Hide comment
@ixti

ixti Oct 20, 2014

Member

Actually, if you'll take a look at sources, Hash#each_key is somewhat similar to hash.each { |k, _| }

Member

ixti commented Oct 20, 2014

Actually, if you'll take a look at sources, Hash#each_key is somewhat similar to hash.each { |k, _| }

@glaucocustodio

This comment has been minimized.

Show comment
Hide comment
@glaucocustodio

glaucocustodio Oct 20, 2014

Contributor

Exactly @ixti

Contributor

glaucocustodio commented Oct 20, 2014

Exactly @ixti

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Oct 20, 2014

Member

Let me know what you come up with. I would be happy to merge any pull requests which make Jekyll faster.

Member

parkr commented Oct 20, 2014

Let me know what you come up with. I would be happy to merge any pull requests which make Jekyll faster.

@glaucocustodio

This comment has been minimized.

Show comment
Hide comment
@glaucocustodio

glaucocustodio Oct 20, 2014

Contributor

Sorry, what do you wanna mean with "what you come up with"? These changes are just to increase performance.

Contributor

glaucocustodio commented Oct 20, 2014

Sorry, what do you wanna mean with "what you come up with"? These changes are just to increase performance.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Oct 20, 2014

Member

Ah, I misread what ixti wrote. Instead of if you'll, I read it as I'll. Mushed together the words.

Thanks.

Member

parkr commented Oct 20, 2014

Ah, I misread what ixti wrote. Instead of if you'll, I read it as I'll. Mushed together the words.

Thanks.

parkr added a commit that referenced this pull request Oct 20, 2014

@parkr parkr merged commit c77d064 into jekyll:master Oct 20, 2014

parkr added a commit that referenced this pull request Oct 20, 2014

@glaucocustodio

This comment has been minimized.

Show comment
Hide comment
@glaucocustodio

glaucocustodio Oct 20, 2014

Contributor

Ok, no problem.

Contributor

glaucocustodio commented Oct 20, 2014

Ok, no problem.

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.