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

Switch to gopkg.in/stack.v1 when available. #34

Closed
ChrisHines opened this issue Nov 6, 2014 · 4 comments
Closed

Switch to gopkg.in/stack.v1 when available. #34

ChrisHines opened this issue Nov 6, 2014 · 4 comments
Assignees

Comments

@ChrisHines
Copy link
Collaborator

The log15/stack package is moving to it's own repository as gopkg.in/stack.v?. This issue is here to remind us to use it when ready.

Also, I'm not sure how to interpret the API compatibility rules for gopkg.in as they relate to nested packages. Clearly log15 does not expose any types from log15/stack, so the main package is free to change its dependency without impacting clients. But what do we do with log15/stack then? I suppose we must keep it available at gopkg.in/inconshreveable/log15.v2/stack in case anyone has used it as a stand alone package. A v3 of log15 could then drop the stack folder going forward.

Thoughts?

@ChrisHines ChrisHines self-assigned this Nov 6, 2014
@inconshreveable
Copy link
Owner

I imagine the API compatibility guarantees apply to all subpackages.

As soon as it's ready, we can go to v3, even if that's the only API change. The nice part about having versions is that we can do it often and not be worried about breaking clients. There's no reason to 'batch' API changes up all together.

@ChrisHines
Copy link
Collaborator Author

Agreed.

As for gopkg.in/stack, it now builds and passes all of its tests on all versions of Go back to 1.0. I'm pretty happy with the API with the exception of CallStack.Format. That one still bugs me a bit.

@ChrisHines
Copy link
Collaborator Author

gopkg.in/stack.v1 is now available. I have pushed branch newstack that uses it. This change breaks the v2 API in four ways.

  1. The exported field Record.CallPC renamed to Call and the type changed from [1]uintptr to stack.Call.
  2. CallerStackHandler renamed to StackHandler.
  3. CallerFileHandler and CallerFuncHandler replaced by CallerHandler which takes a format argument in the same fashion as StackHandler.
  4. Delete package log15/stack.

Using the new stack package incurs some performance trade offs, but keep in mind that the old stack package takes some short cuts that we have since learned are not valid in all cases. The new stack package aims for correctness. As a result the minimal overhead of creating a log record has gone up. On the other hand the Format method of the new stack package is faster, so we gain some of the time back, but only when using one of the stack based handlers. The base overhead is paid by all users.

benchmark                      old ns/op     new ns/op     delta
BenchmarkStreamNoCtx           11828         11977         +1.26%
BenchmarkDiscard               1533          2156          +40.64%
BenchmarkCallerFileHandler     5007          4355          -13.02%
BenchmarkCallerFuncHandler     4409          4399          -0.23%
BenchmarkLogfmtNoCtx           7884          8120          +2.99%
BenchmarkJsonNoCtx             4973          4709          -5.31%
BenchmarkMultiLevelFilter      1644          2218          +34.91%
BenchmarkDescendant1           1558          2211          +41.91%
BenchmarkDescendant2           1625          2144          +31.94%
BenchmarkDescendant4           1648          2241          +35.98%
BenchmarkDescendant8           1661          2270          +36.66%

benchmark                      old allocs     new allocs     delta
BenchmarkStreamNoCtx           23             23             +0.00%
BenchmarkDiscard               1              1              +0.00%
BenchmarkCallerFileHandler     9              8              -11.11%
BenchmarkCallerFuncHandler     7              8              +14.29%
BenchmarkLogfmtNoCtx           22             22             +0.00%
BenchmarkJsonNoCtx             14             14             +0.00%
BenchmarkMultiLevelFilter      1              1              +0.00%
BenchmarkDescendant1           1              1              +0.00%
BenchmarkDescendant2           1              1              +0.00%
BenchmarkDescendant4           1              1              +0.00%
BenchmarkDescendant8           1              1              +0.00%

benchmark                      old bytes     new bytes     delta
BenchmarkStreamNoCtx           901           917           +1.78%
BenchmarkDiscard               128           144           +12.50%
BenchmarkCallerFileHandler     258           265           +2.71%
BenchmarkCallerFuncHandler     282           265           -6.03%
BenchmarkLogfmtNoCtx           772           772           +0.00%
BenchmarkJsonNoCtx             792           792           +0.00%
BenchmarkMultiLevelFilter      128           144           +12.50%
BenchmarkDescendant1           128           144           +12.50%
BenchmarkDescendant2           128           144           +12.50%
BenchmarkDescendant4           128           144           +12.50%
BenchmarkDescendant8           128           144           +12.50%

@ChrisHines
Copy link
Collaborator Author

Closed by #79.

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

No branches or pull requests

2 participants