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

[WIP] Extendability #41

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Conversation

coss
Copy link

@coss coss commented Jun 12, 2015

Earlier this year I wanted to benchmark my internal Router implementation with your benchmarking suite. I looked for a way to disable other Routers, so I don't have to go get the packages. But as everything was basicly implemented in one big file, there was no way to avoid these dependencies. So I split everything into seperate files with build tags. During this step I discovered that not all implementations work correctly (you fixed most of them as you included a simple test case), which led me to implement a new benchmark runner.

The new benchmark and test drivers allow for better extendability. Benchmarks are split into different types of benchmarks, e.g. ParameterReadWrite which benchmarks the reading and writing of a named path segment. These types are the only shared information between benchmarks and Router implementations. A Router implementor specifies the types of benchmarks a Router can run as well as other metadata, such as Router name and homepage URL, during registration. Further allows the new driver implementation to run tests for each benchmark, which uncovered more errors than the current test implementation.

Errors found by the new test driver:

  • Possum. router.Simple is not sufficient for dynamic path segment routing. Further makes Possum’s current implementation of router.RegEx it impossible to benchmark ParameterReadWrite.

Also, I removed the RegEx call inside Beego's constructor - it does not interfere with the benchmarks but is still unnecessary.

Todo:

  • Add CLI usage info
  • Fix highlighting algorithm
  • Refactor runner.go as it is quit messy
  • Add RegEx based white- or blacklisting (prefix with ! to invert) for Router
  • Move the driver inside the new internal package (Go 1.5)

coss added 18 commits June 12, 2015 17:53
The runner.go file is the new benchmark entry point.

The implementation is currently neither feature complete nor completely
correct, which will be fixed and refactored in a following commit.
The new benchmark and test drivers allow for better extendability.
Benchmarks are split into different types of benchmarks, e.g.
ParameterReadWrite which benchmarks the reading and writing of a named
path segment. These types are the only shared information between
benchmarks and Router implementations. A Router implementor specifies
the types of benchmarks a Router can run as well as other metadata,
such as Router name and homepage URL, during registration. Further
allows the new driver implementation to run tests for each benchmark,
which uncovered more errors than the current test implementation.
Refactored benchmark fixtures to use the new driver and implemented
test cases for each benchmark.
The Possum implementation was previously broken. Possum’s router.Simple
is not sufficient for dynamic path segment routing. Further makes
Possum’s current implementation of router.RegEx it impossible to
benchmark ParameterReadWrite.

ParameterReadWrite should be re-enabled once Possum implements named
RegEx groups.
Revels implementation contains now more internals directly copied from
the source code because without it the tests fail constantly with „nil
pointer dereference“ errors.
Zeus is currently disabled. Enable again once daryl/zeus#2 is fixed.
@coss
Copy link
Author

coss commented Jun 12, 2015

Here is a preview of the generated markdown and CLI output. Note that the highlighting algorithm is broken.

@julienschmidt
Copy link
Owner

Awesome! I haven't looked over all your changes yet, but the idea is great.

You can already run certain benchmarks only with e.g. go test -bench="HttpRouter|Gin", but yes, you still need all the dependencies (just go get -u github.com/julienschmidt/go-http-routing-benchmark really)

@coss
Copy link
Author

coss commented Jun 13, 2015

You can already run certain benchmarks only with e.g. go test -bench="HttpRouter|Gin"

Yes, I know. I switched PCs quite a bit at that time and I tried to avoid downloading all dependencies on every machine.

~~While I ran the benchmarks I noticed that most of the execution time (~50%-60%) seems to come from invoking the GC to calculate the memory consumption of a Router object. Changing Fixtures to not contain a pointer may reduce the GC time significantly because the collector will not scan slices, channels, and as of 1.5 (CL 3288), maps if they don´t contain any pointers.~~

Note that even with the new concurrent GC in 1.5, invoking runtime.GC() still causes a STW collection.

I profiled the benchmark runner and found that the overhead in relation to the ideal execution time is not from the GC, which runs in 2ms to 5ms. The estimator in the testing package is sometimes way off. I observed the benchmark running from 1.7s to 3.1s, with some rare outlier running for 5s to 10s, for -time=1s.

@coss coss closed this Jun 21, 2015
@coss coss deleted the feature/extendability branch June 21, 2015 15:46
@julienschmidt
Copy link
Owner

?

@coss coss restored the feature/extendability branch June 21, 2015 15:53
@coss
Copy link
Author

coss commented Jun 21, 2015

Whoops, delete the wrong branch and thanks to wrong aliasing on the current machine I pushed it upstream.

@coss coss reopened this Jun 21, 2015
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