In CL 206757 I added a log.Printf to identify when GOROOT is not read-only.
However, it interacts badly with test sharding in the builders:
the log is repeated for every shard.
Since the log statement isn't particularly high-value, just remove it.
Updates #30316
Change-Id: I385a7f35da59e38ad8b9beef92dc11af931d9571
Reviewed-on: https://go-review.googlesource.com/c/go/+/206898
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Not a fix, but will give us more info when it flakes again.
Updates #35113
Change-Id: I2f90c24530c1bea81dd9d8c7a59f4b0640dfa4c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/206819
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Provide initial linker support for riscv64.
Based on riscv-go port.
Updates #27532
Change-Id: I8a881ce41cd49efef0358bad9171d4d18aaf7ab2
Reviewed-on: https://go-review.googlesource.com/c/go/+/204624
Run-TryBot: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The problem should be fixed by the previous CL. Reenable async
preemption on darwin/arm64.
Updates #35439.
Change-Id: I93e8c4702b4d8fe6abaa6fc9c27def5c8aed1b59
Reviewed-on: https://go-review.googlesource.com/c/go/+/206419
Reviewed-by: Keith Randall <khr@golang.org>
iOS does not support SA_ONSTACK. The signal handler runs on the
G stack. Any writes below the SP may be clobbered by the signal
handler (even without call injection). So we save LR after
decrementing SP on iOS.
Updates #35439.
Change-Id: Ia6d7a0669e0bcf417b44c031d2e26675c1184165
Reviewed-on: https://go-review.googlesource.com/c/go/+/206418
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Some, but not all, architectures mix in OS-provided random seeds when
initializing the fastrand state. The others have TODOs saying we need
to do the same. Lift that logic up in the architecture-independent
part, and use memhash to mix the seed instead of a simple addition.
Previously, dumping the fastrand state at initialization would yield
something like the following on linux-amd64, where the values in the
first column do not change between runs (as thread IDs are sequential
and always start at 0), and the values in the second column, while
changing every run, are pretty correlated:
first run:
0x0 0x44d82f1c
0x5f356495 0x44f339de
0xbe6ac92a 0x44f91cd8
0x1da02dbf 0x44fd91bc
0x7cd59254 0x44fee8a4
0xdc0af6e9 0x4547a1e0
0x3b405b7e 0x474c76fc
0x9a75c013 0x475309dc
0xf9ab24a8 0x4bffd075
second run:
0x0 0xa63fc3eb
0x5f356495 0xa6648dc2
0xbe6ac92a 0xa66c1c59
0x1da02dbf 0xa671bce8
0x7cd59254 0xa70e8287
0xdc0af6e9 0xa7129d2e
0x3b405b7e 0xa7379e2d
0x9a75c013 0xa7e4c64c
0xf9ab24a8 0xa7ecce07
With this change, we get initial states that appear to be much more
unpredictable, both within the same run as well as between runs:
0x11bddad7 0x97241c63
0x553dacc6 0x2bcd8523
0x62c01085 0x16413d92
0x6f40e9e6 0x7a138de6
0xa4898053 0x70d816f0
0x5ca5b433 0x188a395b
0x62778ca9 0xd462c3b5
0xd6e160e4 0xac9b4bd
0xb9571d65 0x597a981d
Change-Id: Ib22c530157d74200df0083f830e0408fd4aaea58
Reviewed-on: https://go-review.googlesource.com/c/go/+/203439
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This test was failing when GOROOT was read-only.
(I'm not sure why that was the case, but it's simpler to convert to to
a script than to try to debug the non-script test.)
Updates #28387
Change-Id: I9943e28d990e5d8b01da10e70531f3ab99e319a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/206897
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There is a (theoretical, but possible) chance that the
random number values a, b used for TestDiv are 0 or 1,
in which case the test would fail.
This CL makes sure that a >= 1 and b >= 2 at all times.
Fixes#35523.
Change-Id: I6451feb94241249516a821cd0066e95a0c65b0ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/206818
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL detects infinite loops due to negative dereference cycles
during escape analysis, and terminates the loop gracefully. We still
fail to print a complete explanation of the escape path, but esc.go
didn't print *any* explanation for these test cases, so the release
blocking issue here is simply that we don't infinite loop.
Updates #35518.
Change-Id: I39beed036e5a685706248852f1fa619af3b7abbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/206619
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also log a message and skip the Chmods if running as root.
Updates #30316
Change-Id: Ifb68d06ce845275a72d64c808407e8609df270bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/206757
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
At least on Darwin notewakeup is not async-signal-safe.
Fixes#35276
Change-Id: I1d7523715e8e77dbd7f21d9b1ed131e52d46cc41
Reviewed-on: https://go-review.googlesource.com/c/go/+/206078
Reviewed-by: Austin Clements <austin@google.com>
If no writes to the package buffer happen, then the package clause
does not get printed. This is a bug for cases where a file just contains
the package clause.
We fix this by separating the printing of package clause to a new
function and calling it from (*pkgBuffer).Write as well as (*Package).flush.
Updates #31457
Change-Id: Ia3bd0ea3963274c460a45d1e37fafc6ee0a197f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/206128
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The current division algorithm produces one word of result at a time,
using 2-word division to compute the top word and mulAddVWW to compute
the remainder. The top word may need to be adjusted by 1 or 2 units.
The recursive version, based on Burnikel, Ziegler, "Fast Recursive Division",
uses the same principles, but in a multi-word setting, so that
multiplication benefits from the Karatsuba algorithm (and possibly later
improvements).
benchmark old ns/op new ns/op delta
BenchmarkDiv/20/10-4 38.2 38.3 +0.26%
BenchmarkDiv/40/20-4 38.7 38.5 -0.52%
BenchmarkDiv/100/50-4 62.5 62.6 +0.16%
BenchmarkDiv/200/100-4 238 259 +8.82%
BenchmarkDiv/400/200-4 311 338 +8.68%
BenchmarkDiv/1000/500-4 604 649 +7.45%
BenchmarkDiv/2000/1000-4 1214 1278 +5.27%
BenchmarkDiv/20000/10000-4 38279 36510 -4.62%
BenchmarkDiv/200000/100000-4 3022057 1359615 -55.01%
BenchmarkDiv/2000000/1000000-4 310827664 54012939 -82.62%
BenchmarkDiv/20000000/10000000-4 33272829421 1965401359 -94.09%
BenchmarkString/10/Base10-4 158 156 -1.27%
BenchmarkString/100/Base10-4 797 792 -0.63%
BenchmarkString/1000/Base10-4 3677 3814 +3.73%
BenchmarkString/10000/Base10-4 16633 17116 +2.90%
BenchmarkString/100000/Base10-4 5779029 1793808 -68.96%
BenchmarkString/1000000/Base10-4 889840820 85524031 -90.39%
BenchmarkString/10000000/Base10-4 134338236860 4935657026 -96.33%
Fixes#21960
Updates #30943
Change-Id: I134c6f81a47870c688ca95b6081eb9211def15a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/172018
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
An error check was missing: If the first argument of a pipeline is
parenthesized, and the pipeline has further arguments, then
syntactically the pipeline is a function invocation and there must
be a "call". Tricky rare corner case, but easily caught.
Add the error check and some tests to verify behavior.
Fixes#31810.
Change-Id: Ica80b7c11284e4ea9e8cc94a01dbbc9a67e42079
Reviewed-on: https://go-review.googlesource.com/c/go/+/206124
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We use the typedValue map to prevent showing typed variables
and constants from appearing in the VARIABLES/CONSTANTS section
because they will be anyways shown in the TYPES section
for that type.
However, when a type is unexported, but the variable is exported,
then unconditionally setting it to true in the map suppresses it
from being shown in the VARIABLES section. Thus, we set the
variable or constant in the typedValue map only when
the type name is exported.
Fixes#31067
Change-Id: Id3ec4b313c9ea7e3ce6fe279680d56f65451719f
Reviewed-on: https://go-review.googlesource.com/c/go/+/206129
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
Reviewed-by: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The cipher suites were apparently renamed late in the standardization
process, and we picked up the legacy name. We can't remove the old
constants, but add correctly named ones.
Fixes#32061
Change-Id: I65ee25c12c10934391af88b76b18565da67453fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/205068
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
TLS 1.3, which requires RSA-PSS, is now enabled without a GODEBUG
opt-out, and with the introduction of
Certificate.SupportedSignatureAlgorithms (#28660) there is a
programmatic way to avoid RSA-PSS (disable TLS 1.3 with MaxVersion and
use that field to specify only PKCS#1 v1.5 SignatureSchemes).
This effectively reverts 0b3a57b537,
although following CL 205061 all of the signing-side logic is
conveniently centralized in signatureSchemesForCertificate.
Fixes#32425
Change-Id: I7c9a8893bb5d518d86eae7db82612b9b2cd257d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/205063
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
This will let applications stop crypto/tls from using a certificate key
with an algorithm that is not supported by its crypto.Signer, like
hardware backed keys that can't do RSA-PSS.
Fixes#28660
Change-Id: I294cc06bddf813fff35c5107540c4a1788e1dace
Reviewed-on: https://go-review.googlesource.com/c/go/+/205062
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Now that we have a full implementation of the logic to check certificate
compatibility, we can let applications just list multiple chains in
Certificates (for example, an RSA and an ECDSA one) and choose the most
appropriate automatically.
NameToCertificate only maps each name to one chain, so simply deprecate
it, and while at it simplify its implementation by not stripping
trailing dots from the SNI (which is specified not to have any, see RFC
6066, Section 3) and by not supporting multi-level wildcards, which are
not a thing in the WebPKI (and in crypto/x509).
The performance of SupportsCertificate without Leaf is poor, but doesn't
affect current users. For now document that, and address it properly in
the next cycle. See #35504.
While cleaning up the Certificates/GetCertificate/GetConfigForClient
behavior, also support leaving Certificates/GetCertificate nil if
GetConfigForClient is set, and send unrecognized_name when there are no
available certificates.
Fixes#29139Fixes#18377
Change-Id: I26604db48806fe4d608388e55da52f34b7ca4566
Reviewed-on: https://go-review.googlesource.com/c/go/+/205059
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Also, add Version to CertificateRequestInfo, as the semantics of
SignatureSchemes change based on version: the ECDSA SignatureSchemes are
only constrained to a specific curve in TLS 1.3.
Fixes#32426
Change-Id: I7a551bea864799e98118349ac2476162893d1ffd
Reviewed-on: https://go-review.googlesource.com/c/go/+/205058
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
We'll also use this function for a better selection logic from
Config.Certificates in a later CL.
Updates #32426
Change-Id: Ie239574d02eb7fd2cf025ec36721c8c7e082d0bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/205057
Reviewed-by: Katie Hockman <katie@golang.org>
This refactors a lot of the certificate support logic to make it cleaner
and reusable where possible. These changes will make the following CLs
much simpler.
In particular, the heavily overloaded pickSignatureAlgorithm is gone.
That function used to cover both signing and verifying side, would work
both for pre-signature_algorithms TLS 1.0/1.1 and TLS 1.2, and returned
sigalg, type and hash.
Now, TLS 1.0/1.1 and 1.2 are differentiated at the caller, as they have
effectively completely different logic. TLS 1.0/1.1 simply use
legacyTypeAndHashFromPublicKey as they employ a fixed hash function and
signature algorithm for each public key type. TLS 1.2 is instead routed
through selectSignatureScheme (on the signing side) or
isSupportedSignatureAlgorithm (on the verifying side) and
typeAndHashFromSignatureScheme, like TLS 1.3.
On the signing side, signatureSchemesForCertificate was already version
aware (for PKCS#1 v1.5 vs PSS support), so selectSignatureScheme just
had to learn the Section 7.4.1.4.1 defaults for a missing
signature_algorithms to replace pickSignatureAlgorithm.
On the verifying side, pickSignatureAlgorithm was also checking the
public key type, while isSupportedSignatureAlgorithm +
typeAndHashFromSignatureScheme are not, but that check was redundant
with the one in verifyHandshakeSignature.
There should be no major change in behavior so far. A few minor changes
came from the refactor: we now correctly require signature_algorithms in
TLS 1.3 when using a certificate; we won't use Ed25519 in TLS 1.2 if the
client didn't send signature_algorithms; and we don't send
ec_points_format in the ServerHello (a compatibility measure) if we are
not doing ECDHE anyway because there are no mutually supported curves.
The tests also got simpler because they test simpler functions. The
caller logic switching between TLS 1.0/1.1 and 1.2 is tested by the
transcript tests.
Updates #32426
Change-Id: Ice9dcaea78d204718f661f8d60efdb408ba41577
Reviewed-on: https://go-review.googlesource.com/c/go/+/205061
Reviewed-by: Katie Hockman <katie@golang.org>
This CL is based on work started by Joe Tsai in CL 94855.
It's rebased on top of the latest master branch, and
addresses various code review comments and findings
from attempting to use the original CL in practice.
The testing package documents a naming convention for examples
so that documentation tools can associate them with:
• a package (Example or Example_suffix)
• a function F (ExampleF or ExampleF_suffix)
• a type T (ExampleT or ExampleT_suffix)
• a method T.M (ExampleT_M or ExampleT_M_suffix)
This naming convention is in widespread use and enforced
via existing go vet checks.
This change adds first-class support for classifying examples
to go/doc, the package responsible for computing package
documentation from Go AST.
There isn't a way to supply test files to New that works well.
External test files may have a package name with "_test" suffix,
so ast.NewPackage may end up using the wrong package name if given
test files. A workaround is to add test files to *ast.Package.Files
after it is returned from ast.NewPackage:
pkg, _ := ast.NewPackage(fset, goFiles, ...)
for name, f := range testGoFiles {
pkg.Files[name] = f
}
p := doc.New(pkg, ...)
But that is not a good API.
After nearly 8 years, a new entry-point is added to the go/doc
package, the function NewFromFiles. It accepts a Go package in
the form of a list of parsed Go files (including _test.go files)
and an import path. The caller is responsible with filtering out
files based on build constraints, as was the case before with New.
NewFromFiles computes package documentation from .go files,
extracts examples from _test.go files and classifies them.
Examples fields are added to Package, Type, and Func. They are
documented to only be populated with examples found in _test.go
files provided to NewFromFiles.
The new behavior is:
1. NewFromFiles computes package documentation from provided
parsed .go files. It extracts examples from _test.go files.
2. It assigns each Example to corresponding Package, Type,
or Func.
3. It sets the Suffix field in each example to the suffix.
4. Malformed examples are skipped.
This change implements behavior that matches the current behavior
of existing godoc-like tools, and will enable them to rely on the
logic in go/doc instead of reimplementing it themselves.
Fixes#23864
Change-Id: Iae834f2ff92fbd1c93a9bb7c2bf47d619bee05cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/204830
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Disable a portion of the TestDWARF testpoint for Windows using
c-archive buildmode, pending investigation of the issue at hand, so as
to get the longtest builder unblocked.
Updates #35512.
Change-Id: Ib72d82ceaa674b9a51da220fb8e225231d5c3433
Reviewed-on: https://go-review.googlesource.com/c/go/+/206557
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This test failed in a pending CL, and I would rather debug it as a script.
Change-Id: I231367c86415ab61d0f9e08b88c9546d32b373b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/206498
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This test failed in a pending CL, and I would rather debug it as a script.
Change-Id: I0ae7486a9949bea40d5dd36afe6919f86f14dfa7
Reviewed-on: https://go-review.googlesource.com/c/go/+/206499
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL restores behavior before CL 189797 and fixes a misleading
comment. modload.ListModules may return info without a version for the
main module and for modules replaced with local directories.
Fixes#35505
Change-Id: I5b4e68053a680ff897b072fdf6e7aa17b6e1ac34
Reviewed-on: https://go-review.googlesource.com/c/go/+/206538
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This allows the target of 'go build' to be a filename constructed
using ioutil.TempFile or similar, without racily deleting the file
before rebuilding it.
Updates #32407
Updates #28387
Change-Id: I4c5072830a02b93f0c4186b50bffa9de00257afe
Reviewed-on: https://go-review.googlesource.com/c/go/+/206477
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Apply the suggestions made in the too-late review of
golang.org/cl/137215
to move the comments to a separate line and use proper
punctuation.
Change-Id: If2b4e5ce8af8c78fa51280d5c87c852a76dae459
Reviewed-on: https://go-review.googlesource.com/c/go/+/206125
Reviewed-by: Robert Griesemer <gri@golang.org>
This makes Ed25519 certificates work for CreateCRL(). This previously
failed (panic: crypto: requested hash function #0 is unavailable) because
the hash could not be skipped, but Ed25519 uses no hash.
A similar fix has been applied in a few other places when Ed25519 was added
when Ed25519 certificates were originally introduced, but was missed
here.
Change-Id: I16fcfcd53ba3bb8f773e5de972b8fedde1f6350e
Change-Id: I16fcfcd53ba3bb8f773e5de972b8fedde1f6350e
GitHub-Last-Rev: bf7f1458f8
GitHub-Pull-Request: golang/go#35241
Reviewed-on: https://go-review.googlesource.com/c/go/+/204046
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
search.CleanPatterns now preserves backslash separators in absolute
paths in Windows. These had resulted in inconsistent error messages.
search.MatchPackagesInFS is now more accepting of patterns with
backslashes. It was inconsistent before.
Several tests are fixed to work with Windows (mostly to match slashes
or backslashes).
Fixes#25300
Change-Id: Ibbf9ccd145353f7e3d345205c6fcc01d7066d1c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/206144
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The problem causing the assert in #21647 are fixed at this point,
along with various other linker issues with plugin + Darwin. With
this in mind, remove the "-ldflags=-w" workaround for plugin mode on
Darwin and re-enable the appropriate tests misc/cgo/testplugin
Fixes#21647.
Fixes#27502.
Change-Id: I5b662987b138b06cfc9e1f9f6d804cf682bd501a
Reviewed-on: https://go-review.googlesource.com/c/go/+/206198
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Assorted fixups in the linker needed to enable turning back on
DWARF generation when building plugins for Darwin. Includes:
- don't suppress import of runtime/cgo in the linker for
Darwin if we are linking in plugin mode
- in calcCompUnitRanges handle the case where we encounter
linker-generated functions that have no associated Unit (and
also have no DWARF)
- generalize a guard in relocsym() include so as to avoid
triggering a spurious error on go.info symbols in plugin mode
Updates #21647.
Updates #27502.
Change-Id: I317fea97bef2f3461e31498e63f9fd6d8b8f4b23
Reviewed-on: https://go-review.googlesource.com/c/go/+/182959
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Otherwise we'd panic with a stack overflow.
Most programs are in control of the data being encoded and can ensure
there are no cycles, but sometimes it's not that simple. For example,
running a user's html template with script tags can easily result in
crashes if the user can find a pointer cycle.
Adding the checks via a map to every ptrEncoder.encode call slowed down
the benchmarks below by a noticeable 13%. Instead, only start doing the
relatively expensive pointer cycle checks if we're many levels of
pointers deep in an encode state.
A threshold of 1000 is small enough to capture pointer cycles before
they're a problem (the goroutine stack limit is currently 1GB, and I
needed close to a million levels to reach it). Yet it's large enough
that reasonable uses of the json encoder only see a tiny 1% slow-down
due to the added ptrLevel field and check.
name old time/op new time/op delta
CodeEncoder-8 2.34ms ± 1% 2.37ms ± 0% +1.05% (p=0.000 n=10+10)
CodeMarshal-8 2.42ms ± 1% 2.44ms ± 0% +1.10% (p=0.000 n=10+10)
name old speed new speed delta
CodeEncoder-8 829MB/s ± 1% 820MB/s ± 0% -1.04% (p=0.000 n=10+10)
CodeMarshal-8 803MB/s ± 1% 795MB/s ± 0% -1.09% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
CodeEncoder-8 43.1kB ± 8% 42.5kB ±10% ~ (p=0.989 n=10+10)
CodeMarshal-8 1.99MB ± 0% 1.99MB ± 0% ~ (p=0.254 n=9+6)
name old allocs/op new allocs/op delta
CodeEncoder-8 0.00 0.00 ~ (all equal)
CodeMarshal-8 1.00 ± 0% 1.00 ± 0% ~ (all equal)
Finally, add a few tests to ensure that the code handles the edge cases
properly.
Fixes#10769.
Change-Id: I73d48e0cf6ea140127ea031f2dbae6e6a55e58b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/187920
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
