Skip to content
This repository was archived by the owner on Nov 6, 2022. It is now read-only.

simplify parsing M-SEARCH method, group P methods #323

Closed
wants to merge 1 commit into from

Conversation

ploxiln
Copy link
Contributor

@ploxiln ploxiln commented Jun 30, 2016

This just makes the method verb parsing code a bit more elegant:

  • can use same switch-lookup for - char case for M-SEARCH
  • move PROPFIND and PURGE to be next to the other P methods

also, change IS_ALPHA(ch) to just A <= ch <= Z
(very slight optimization, only uppercase will match in switch)

I tested with ./bench on OS X and in a Linux VM, and performance is pretty much the same with this change, just an insignificant smidge faster. I tested branches in back/forth/back/forth pattern.

OS X,     master     :  830174.250000 req/sec,  829930.875000 req/sec
OS X,     this branch:  841326.375000 req/sec,  836593.812500 req/sec

Linux VM, master     : 1164776.500000 req/sec, 1165652.750000 req/sec
Linux VM, this branch: 1179126.250000 req/sec, 1175704.875000 req/sec

But this isn't about the performance, just mostly about getting rid of the extra if statement just for the M-Search method.

can use same switch-lookup for '-' char case
move PROPFIND and PURGE to be next to the other P methods

change IS_ALPHA(ch) to  A <= ch <= Z
(very slight optimization, only uppercase will match in switch)
@@ -1007,7 +1007,7 @@ size_t http_parser_execute (http_parser *parser,
UPDATE_STATE(s_req_spaces_before_url);
} else if (ch == matcher[parser->index]) {
; /* nada */
} else if (IS_ALPHA(ch)) {
} else if ((ch >= 'A' && ch <= 'Z') || ch == '-') {

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@indutny
Copy link
Member

indutny commented Jun 30, 2016

LGTM.

@jasnell or @mscdex : do you guys want to sign off?

@ploxiln
Copy link
Contributor Author

ploxiln commented Jul 1, 2016

I am willing to make adjustments if desired, fwiw.

@indutny
Copy link
Member

indutny commented Jul 1, 2016

@ploxiln there is no need in this! I'll land it in 24 hours if no one will reply.

@ploxiln
Copy link
Contributor Author

ploxiln commented Oct 6, 2016

friendly ping 😁

@jasnell
Copy link
Member

jasnell commented Oct 6, 2016

O.o just saw this! sorry about that... LGTM if @indutny is happy with it.

@ploxiln ploxiln mentioned this pull request Jun 12, 2017
indutny pushed a commit that referenced this pull request Jun 14, 2017
can use same switch-lookup for '-' char case
move PROPFIND and PURGE to be next to the other P methods

change IS_ALPHA(ch) to  A <= ch <= Z
(very slight optimization, only uppercase will match in switch)

PR-URL: #323
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@indutny
Copy link
Member

indutny commented Jun 14, 2017

Landed in 1b79aba, thank you! Sorry for very very very long delay 😭

@indutny indutny closed this Jun 14, 2017
shekhei pushed a commit to shekhei/http-parser that referenced this pull request Sep 19, 2017
can use same switch-lookup for '-' char case
move PROPFIND and PURGE to be next to the other P methods

change IS_ALPHA(ch) to  A <= ch <= Z
(very slight optimization, only uppercase will match in switch)

PR-URL: nodejs#323
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: James M Snell <[email protected]>
shekhei pushed a commit to shekhei/http-parser that referenced this pull request Sep 19, 2017
can use same switch-lookup for '-' char case
move PROPFIND and PURGE to be next to the other P methods

change IS_ALPHA(ch) to  A <= ch <= Z
(very slight optimization, only uppercase will match in switch)

PR-URL: nodejs#323
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants