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

http2: refactor counter in mapToHeaders inner loop #32012

Closed
wants to merge 1 commit into from

Conversation

mkrawczuk
Copy link
Contributor

@mkrawczuk mkrawczuk commented Feb 28, 2020

This cosmetic change is to prevent potential bugs - e.g., someone might
automatically use the variable k, instead of key, that is used in
vicinity of this loop. Got rid of old-style var declaration. It does
make sense to sometimes use var in for loops for optimization, but
not in this context. Also changed post-increment to pre-increment in
iteration steps. It is probably done by the optimizer anyway,
otherwise it will save an opcode each iteration. And it is a good
practice.

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

This change is to prevent potential bugs - e.g., someone might
automatically use the variable `k` instead of `key`, that is used in
vicinity of this loop.
Also changed postincrement to preincrement in iteration steps. It is
probably done by the optimizer anyway, but otherwise it will save an
opcode each iteration. And it is a good practice.
@nodejs-github-bot nodejs-github-bot added the http2 Issues or PRs related to the http2 subsystem. label Feb 28, 2020
@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 9, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@addaleax
Copy link
Member

addaleax commented Mar 13, 2020

Landed in 4ffb39b 🎉

@addaleax addaleax closed this Mar 13, 2020
addaleax pushed a commit that referenced this pull request Mar 13, 2020
This change is to prevent potential bugs - e.g., someone might
automatically use the variable `k` instead of `key`, that is used in
vicinity of this loop.
Also changed postincrement to preincrement in iteration steps. It is
probably done by the optimizer anyway, but otherwise it will save an
opcode each iteration. And it is a good practice.

PR-URL: #32012
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BridgeAR pushed a commit that referenced this pull request Mar 17, 2020
This change is to prevent potential bugs - e.g., someone might
automatically use the variable `k` instead of `key`, that is used in
vicinity of this loop.
Also changed postincrement to preincrement in iteration steps. It is
probably done by the optimizer anyway, but otherwise it will save an
opcode each iteration. And it is a good practice.

PR-URL: #32012
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins MylesBorins mentioned this pull request Mar 19, 2020
MylesBorins pushed a commit that referenced this pull request Mar 24, 2020
This change is to prevent potential bugs - e.g., someone might
automatically use the variable `k` instead of `key`, that is used in
vicinity of this loop.
Also changed postincrement to preincrement in iteration steps. It is
probably done by the optimizer anyway, but otherwise it will save an
opcode each iteration. And it is a good practice.

PR-URL: #32012
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Apr 22, 2020
This change is to prevent potential bugs - e.g., someone might
automatically use the variable `k` instead of `key`, that is used in
vicinity of this loop.
Also changed postincrement to preincrement in iteration steps. It is
probably done by the optimizer anyway, but otherwise it will save an
opcode each iteration. And it is a good practice.

PR-URL: #32012
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants