Improves the performance of getHandle() when routing requests.

[atheriel]

This PR contains performance tweaks to the internal getHandle() function used inside of the router, which in turn calls the canServe() method for each handler until it finds one that can take the request. Since this function is essentially "overhead" on every request, it is ideal if it is fast.

The PR makes the following changes:

• Replaces the use of stri_match() with stri_match_first_regex() directly, removing the expensive and unnecessary calls to match.arg().

• Imports stri_match_first_regex() into the environment to remove the need to call ::, which was taking up about 25% of the execution time.

• Takes advantage of the fact that seq can be NULL in for loops to simplify the getHandle() function.

This reduces the overhead of getHandle() by about 50%. From my own testing, this function moved from about 20% of program execution time down to 12% for simple endpoints in our production APIs; for more complex endpoints it moved down from 4% to 2%.

PR task list:

• Update NEWS
• Add tests
• Update documentation with devtools::document()

This is not a public-facing change, so I don't think it needs an entry in the NEWS.md file, but I can add one if you disagree. Nor does it really require new tests -- existing integration tests should cover these changes.

[CLAassistant]

All committers have signed the CLA.

[antoine-sachet]

Not that my opinion on this matters much, but this is definitely NEWS-worthy. This is what the "improvements" section is for! Like you, I work on production APIs and a 10% speed-up for simple endpoints is great.

Also I would say the change is public-facing insofar as there will be a measurable effect on all existing plumber APIs.

[schloerke]

Do you mind sharing how you loadtest or determine the performance increase? Thank you!

[atheriel]

Sure, although I think how I actually did it might be a bit more involved than actually necessary to show the difference:

• I generated artificial (although realistic) heavy load with wrk. For the test against our production code I ran the API on one server and wrk on another server, but I have run similar tests fully locally.

• To sample stack traces under this load I used Rprof() (and probably my own external Rprof tool as well, can't quite remember).

• I used a homebrew R script (which you can find in the linked repo above) to aggregate the Rprof() output and Brendon Gregg's flamegraph.pl to visualize it. I can't share the original visualizations because they contain potentially sensitive function names, but you can get an idea of what they looked like by checking out an issue I filed against httpuv.

[schloerke]

Thank you. I should be able to repeat something similar on my end.

[atheriel]

A much simpler way, now that I think about it, is just to test the throughput of a sample API before and after the change.

[schloerke]

Correct. The before and after check works for validating the speed up.

Sounds like you use something similar to profvis for inspecting slow code sections.

—————

@blairj09 and I have been looking into future integrations for plumber to make sure the codebase doesn’t get slower with a particular pr. Currently we have been looking at ab due to ease of use / availability. Also looking into the loadtest R package.

[atheriel]

I'm (obviously) pretty interested in performance regressions as well, and would be happy to see that kind of work.

[meztez]

I would still keep the check on NULL handlers in the route function. Checking for null handlers takes about nothing; looping on a NULL takes more. Performance are increase on valid call but you would decrease performance on NULL handlers. Still great catch.

> a <- expression(for (h in NULL) {}); b <- expression(if (!is.null(NULL)) {})
> microbenchmark::microbenchmark(a, b)
# Unit: nanoseconds
#  expr min lq  mean median uq  max neval
#    a   0  0 26.11      1  1 2553   100
#    b   0  0  0.47      0  1    1   100

[cpsievert]

The stringi change makes sense, but I don't follow the logic behind the change in getHandle() (how is seq() involved)? Also, I don't see such drastic differences in the minimal example in #484 (comment)

[meztez]

The stringi change makes sense, but I don't follow the logic behind the change in getHandle() (how is seq() involved)? Also, I don't see such drastic differences in the minimal example in #484 (comment)

It means it is valid to loop on a NULL value in R. seq is in reference to the basic for loop structure for (i in seq). But as shown above, this should be avoided as it takes longer than checking for a NULL value directly because it still has to initialize the loop structure.

tldr : getHandle does not need to be changed.