| Age | Commit message (Collapse) | Author |
|---|
|
Currently, io.ReadAll allocates a significant amount of intermediate
memory as it grows its result slice to the size of the input data.
This CL aims to reduce the allocated memory. Geomean benchstat results
comparing existing io.ReadAll to this CL for a variety of input sizes:
│ old | new vs base │
sec/op 132.2µ 66.32µ -49.83%
B/op 645.4Ki 324.6Ki -49.70%
final-capacity 178.3k 151.3k -15.10%
excess-ratio 1.216 1.033 -15.10%
The corresponding full benchstat results are below. The input data sizes
are a blend of random sizes, power-of-2 sizes, and power-of-10 sizes.
This CL reduces intermediate bytes allocated in io.ReadAll by reading
via a set of slices of exponentially growing size, and then copying
into a final perfectly-sized slice at the end.
The current memory allocations impact real uses. For example, in #50774
two real-world reports were ~60% more bytes allocated via io.ReadAll
compared to an alternate approach, and also a separate report of
~5x more bytes allocated than the input data size of ~5MiB.
Separately, bytes.Buffer.ReadFrom uses a 2x growth strategy, which
usually can beat the pre-existing io.ReadAll on total bytes allocated
but sometimes not (depending on alignment between exact input data size
and growth). That said, bytes.Buffer.ReadFrom usually ends up with
more excess memory used in a larger final result than the current
io.ReadAll (often significantly more).
If we compare bytes.Buffer.ReadFrom to this CL, we also see
better geomean overall results reported with this CL:
│ bytes.Buffer | io.ReadAll (new) │
sec/op 104.6µ 66.32µ -36.60%
B/op 466.9Ki 324.6Ki -30.48%
final-capacity 247.4k 151.3k -38.84%
excess-ratio 1.688 1.033 -38.84%
(Full corresponding benchstat results comparing this CL vs. bytes.Buffer
are at https://go.dev/play/p/eqwk2BkaSwJ).
One challenge with almost any change of growth strategy for something
widely used is there can be a subset of users that benefited more from
the old growth approach (e.g., based on their data size aligning
particularly well with the old growth), even if the majority of users
on average benefit from the new growth approach.
To help mitigate that, this CL somewhat follows the old read pattern
in its early stages.
Here are the full benchstat results comparing the existing
io.ReadAll vs. this CL. The standard metrics are included, plus
the final result capacity and an excess capacity ratio, which is
the final capacity of the result divided by the input data size (so 1.0
is no excess memory present in the result, though due to
size class rounding the ratio is usually above 1.0 unless the
input data size exactly matches a size class).
We consider smaller reported excess capacity to be better for most
uses given it means the final allocation puts less pressure on the GC
(both in cases when it will almost immediately be garbage in user code,
or if for example the final result is held for multiple GC cycles).
The input data sizes used in the benchmarks:
- Six powers of 10.
- Six powers of 2.
- Ten random sizes between 1KiB and 100MiB (chosen uniformly
on a log scale).
- size=300 (so that we have something below 512, which is the
initial read size).
goos: linux
goarch: amd64
pkg: io
cpu: AMD EPYC 7B13
│ old │ io.ReadAll (new) │
│ sec/op │ sec/op vs base │
ReadAll/size=300-16 113.0n ± 0% 115.4n ± 2% +2.08% (p=0.005 n=20)
ReadAll/size=512-16 295.0n ± 2% 288.7n ± 1% -2.14% (p=0.006 n=20)
ReadAll/size=1000-16 549.2n ± 1% 492.8n ± 1% -10.28% (p=0.000 n=20)
ReadAll/size=4096-16 3.193µ ± 1% 2.277µ ± 1% -28.70% (p=0.000 n=20)
ReadAll/size=6648-16 4.318µ ± 1% 3.100µ ± 1% -28.21% (p=0.000 n=20)
ReadAll/size=10000-16 7.771µ ± 1% 4.629µ ± 1% -40.43% (p=0.000 n=20)
ReadAll/size=12179-16 7.724µ ± 1% 5.066µ ± 1% -34.42% (p=0.000 n=20)
ReadAll/size=16384-16 13.664µ ± 1% 7.309µ ± 1% -46.51% (p=0.000 n=20)
ReadAll/size=32768-16 24.07µ ± 2% 14.52µ ± 2% -39.67% (p=0.000 n=20)
ReadAll/size=65536-16 43.14µ ± 2% 24.00µ ± 2% -44.37% (p=0.000 n=20)
ReadAll/size=80000-16 57.12µ ± 2% 31.28µ ± 2% -45.24% (p=0.000 n=20)
ReadAll/size=100000-16 75.08µ ± 2% 38.18µ ± 3% -49.15% (p=0.000 n=20)
ReadAll/size=118014-16 76.06µ ± 1% 50.03µ ± 3% -34.22% (p=0.000 n=20)
ReadAll/size=131072-16 103.99µ ± 1% 52.31µ ± 2% -49.70% (p=0.000 n=20)
ReadAll/size=397601-16 518.1µ ± 6% 204.2µ ± 2% -60.58% (p=0.000 n=20)
ReadAll/size=626039-16 934.9µ ± 3% 398.7µ ± 7% -57.35% (p=0.000 n=20)
ReadAll/size=1000000-16 1800.3µ ± 8% 651.4µ ± 6% -63.82% (p=0.000 n=20)
ReadAll/size=1141838-16 2236.3µ ± 5% 710.2µ ± 5% -68.24% (p=0.000 n=20)
ReadAll/size=2414329-16 4.517m ± 3% 1.471m ± 3% -67.43% (p=0.000 n=20)
ReadAll/size=5136407-16 8.547m ± 3% 2.060m ± 1% -75.90% (p=0.000 n=20)
ReadAll/size=10000000-16 13.303m ± 4% 3.767m ± 4% -71.68% (p=0.000 n=20)
ReadAll/size=18285584-16 23.414m ± 2% 6.790m ± 5% -71.00% (p=0.000 n=20)
ReadAll/size=67379426-16 55.93m ± 4% 24.50m ± 5% -56.20% (p=0.000 n=20)
ReadAll/size=100000000-16 84.61m ± 5% 33.84m ± 5% -60.00% (p=0.000 n=20)
geomean 132.2µ 66.32µ -49.83%
│ old │ io.ReadAll (new) │
│ B/op │ B/op vs base │
ReadAll/size=300-16 512.0 ± 0% 512.0 ± 0% ~ (p=1.000 n=20) ¹
ReadAll/size=512-16 1.375Ki ± 0% 1.250Ki ± 0% -9.09% (p=0.000 n=20)
ReadAll/size=1000-16 2.750Ki ± 0% 2.125Ki ± 0% -22.73% (p=0.000 n=20)
ReadAll/size=4096-16 17.00Ki ± 0% 10.12Ki ± 0% -40.44% (p=0.000 n=20)
ReadAll/size=6648-16 23.75Ki ± 0% 15.75Ki ± 0% -33.68% (p=0.000 n=20)
ReadAll/size=10000-16 45.00Ki ± 0% 23.88Ki ± 0% -46.94% (p=0.000 n=20)
ReadAll/size=12179-16 45.00Ki ± 0% 25.88Ki ± 0% -42.50% (p=0.000 n=20)
ReadAll/size=16384-16 82.25Ki ± 0% 36.88Ki ± 0% -55.17% (p=0.000 n=20)
ReadAll/size=32768-16 150.25Ki ± 0% 78.88Ki ± 0% -47.50% (p=0.000 n=20)
ReadAll/size=65536-16 278.3Ki ± 0% 134.9Ki ± 0% -51.53% (p=0.000 n=20)
ReadAll/size=80000-16 374.3Ki ± 0% 190.9Ki ± 0% -49.00% (p=0.000 n=20)
ReadAll/size=100000-16 502.3Ki ± 0% 214.9Ki ± 0% -57.22% (p=0.000 n=20)
ReadAll/size=118014-16 502.3Ki ± 0% 286.9Ki ± 0% -42.88% (p=0.000 n=20)
ReadAll/size=131072-16 670.3Ki ± 0% 294.9Ki ± 0% -56.01% (p=0.000 n=20)
ReadAll/size=397601-16 1934.3Ki ± 0% 919.8Ki ± 0% -52.45% (p=0.000 n=20)
ReadAll/size=626039-16 3.092Mi ± 0% 1.359Mi ± 0% -56.04% (p=0.000 n=20)
ReadAll/size=1000000-16 4.998Mi ± 0% 2.086Mi ± 0% -58.27% (p=0.000 n=20)
ReadAll/size=1141838-16 6.334Mi ± 0% 2.219Mi ± 0% -64.98% (p=0.000 n=20)
ReadAll/size=2414329-16 12.725Mi ± 0% 4.789Mi ± 0% -62.37% (p=0.000 n=20)
ReadAll/size=5136407-16 25.28Mi ± 0% 10.44Mi ± 0% -58.71% (p=0.000 n=20)
ReadAll/size=10000000-16 49.84Mi ± 0% 21.92Mi ± 0% -56.02% (p=0.000 n=20)
ReadAll/size=18285584-16 97.88Mi ± 0% 35.99Mi ± 0% -63.23% (p=0.000 n=20)
ReadAll/size=67379426-16 375.2Mi ± 0% 158.0Mi ± 0% -57.91% (p=0.000 n=20)
ReadAll/size=100000000-16 586.7Mi ± 0% 235.9Mi ± 0% -59.80% (p=0.000 n=20)
geomean 645.4Ki 324.6Ki -49.70%
│ old │ io.ReadAll (new) │
│ final-cap │ final-cap vs base │
ReadAll/size=300-16 512.0 ± 0% 512.0 ± 0% ~ (p=1.000 n=20) ¹
ReadAll/size=512-16 896.0 ± 0% 512.0 ± 0% -42.86% (p=0.000 n=20)
ReadAll/size=1000-16 1.408k ± 0% 1.024k ± 0% -27.27% (p=0.000 n=20)
ReadAll/size=4096-16 5.376k ± 0% 4.096k ± 0% -23.81% (p=0.000 n=20)
ReadAll/size=6648-16 6.912k ± 0% 6.784k ± 0% -1.85% (p=0.000 n=20)
ReadAll/size=10000-16 12.29k ± 0% 10.24k ± 0% -16.67% (p=0.000 n=20)
ReadAll/size=12179-16 12.29k ± 0% 12.29k ± 0% ~ (p=1.000 n=20) ¹
ReadAll/size=16384-16 21.76k ± 0% 16.38k ± 0% -24.71% (p=0.000 n=20)
ReadAll/size=32768-16 40.96k ± 0% 32.77k ± 0% -20.00% (p=0.000 n=20)
ReadAll/size=65536-16 73.73k ± 0% 65.54k ± 0% -11.11% (p=0.000 n=20)
ReadAll/size=80000-16 98.30k ± 0% 81.92k ± 0% -16.67% (p=0.000 n=20)
ReadAll/size=100000-16 131.1k ± 0% 106.5k ± 0% -18.75% (p=0.000 n=20)
ReadAll/size=118014-16 131.1k ± 0% 122.9k ± 0% -6.25% (p=0.000 n=20)
ReadAll/size=131072-16 172.0k ± 0% 131.1k ± 0% -23.81% (p=0.000 n=20)
ReadAll/size=397601-16 442.4k ± 0% 401.4k ± 0% -9.26% (p=0.000 n=20)
ReadAll/size=626039-16 704.5k ± 0% 630.8k ± 0% -10.47% (p=0.000 n=20)
ReadAll/size=1000000-16 1.114M ± 0% 1.008M ± 0% -9.56% (p=0.000 n=20)
ReadAll/size=1141838-16 1.401M ± 0% 1.147M ± 0% -18.13% (p=0.000 n=20)
ReadAll/size=2414329-16 2.753M ± 0% 2.417M ± 0% -12.20% (p=0.000 n=20)
ReadAll/size=5136407-16 5.399M ± 0% 5.145M ± 0% -4.70% (p=0.000 n=20)
ReadAll/size=10000000-16 10.56M ± 0% 10.00M ± 0% -5.28% (p=0.000 n=20)
ReadAll/size=18285584-16 20.65M ± 0% 18.29M ± 0% -11.42% (p=0.000 n=20)
ReadAll/size=67379426-16 78.84M ± 0% 67.39M ± 0% -14.53% (p=0.000 n=20)
ReadAll/size=100000000-16 123.2M ± 0% 100.0M ± 0% -18.82% (p=0.000 n=20)
geomean 178.3k 151.3k -15.10%
│ old │ io.ReadAll (new) │
│ excess-ratio │ excess-ratio vs base │
ReadAll/size=300-16 1.707 ± 0% 1.707 ± 0% ~ (p=1.000 n=20) ¹
ReadAll/size=512-16 1.750 ± 0% 1.000 ± 0% -42.86% (p=0.000 n=20)
ReadAll/size=1000-16 1.408 ± 0% 1.024 ± 0% -27.27% (p=0.000 n=20)
ReadAll/size=4096-16 1.312 ± 0% 1.000 ± 0% -23.78% (p=0.000 n=20)
ReadAll/size=6648-16 1.040 ± 0% 1.020 ± 0% -1.92% (p=0.000 n=20)
ReadAll/size=10000-16 1.229 ± 0% 1.024 ± 0% -16.68% (p=0.000 n=20)
ReadAll/size=12179-16 1.009 ± 0% 1.009 ± 0% ~ (p=1.000 n=20) ¹
ReadAll/size=16384-16 1.328 ± 0% 1.000 ± 0% -24.70% (p=0.000 n=20)
ReadAll/size=32768-16 1.250 ± 0% 1.000 ± 0% -20.00% (p=0.000 n=20)
ReadAll/size=65536-16 1.125 ± 0% 1.000 ± 0% -11.11% (p=0.000 n=20)
ReadAll/size=80000-16 1.229 ± 0% 1.024 ± 0% -16.68% (p=0.000 n=20)
ReadAll/size=100000-16 1.311 ± 0% 1.065 ± 0% -18.76% (p=0.000 n=20)
ReadAll/size=118014-16 1.111 ± 0% 1.041 ± 0% -6.30% (p=0.000 n=20)
ReadAll/size=131072-16 1.312 ± 0% 1.000 ± 0% -23.78% (p=0.000 n=20)
ReadAll/size=397601-16 1.113 ± 0% 1.010 ± 0% -9.25% (p=0.000 n=20)
ReadAll/size=626039-16 1.125 ± 0% 1.008 ± 0% -10.40% (p=0.000 n=20)
ReadAll/size=1000000-16 1.114 ± 0% 1.008 ± 0% -9.52% (p=0.000 n=20)
ReadAll/size=1141838-16 1.227 ± 0% 1.004 ± 0% -18.17% (p=0.000 n=20)
ReadAll/size=2414329-16 1.140 ± 0% 1.001 ± 0% -12.19% (p=0.000 n=20)
ReadAll/size=5136407-16 1.051 ± 0% 1.002 ± 0% -4.66% (p=0.000 n=20)
ReadAll/size=10000000-16 1.056 ± 0% 1.000 ± 0% -5.30% (p=0.000 n=20)
ReadAll/size=18285584-16 1.129 ± 0% 1.000 ± 0% -11.43% (p=0.000 n=20)
ReadAll/size=67379426-16 1.170 ± 0% 1.000 ± 0% -14.53% (p=0.000 n=20)
ReadAll/size=100000000-16 1.232 ± 0% 1.000 ± 0% -18.83% (p=0.000 n=20)
geomean 1.216 1.033 -15.10%
│ io.ReadAll │ io.ReadAll (new) │
│ allocs/op │ allocs/op vs base │
ReadAll/size=300-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=20) ¹
ReadAll/size=512-16 2.000 ± 0% 3.000 ± 0% +50.00% (p=0.000 n=20)
ReadAll/size=1000-16 3.000 ± 0% 4.000 ± 0% +33.33% (p=0.000 n=20)
ReadAll/size=4096-16 7.000 ± 0% 9.000 ± 0% +28.57% (p=0.000 n=20)
ReadAll/size=6648-16 8.000 ± 0% 10.000 ± 0% +25.00% (p=0.000 n=20)
ReadAll/size=10000-16 10.00 ± 0% 11.00 ± 0% +10.00% (p=0.000 n=20)
ReadAll/size=12179-16 10.00 ± 0% 11.00 ± 0% +10.00% (p=0.000 n=20)
ReadAll/size=16384-16 12.00 ± 0% 13.00 ± 0% +8.33% (p=0.000 n=20)
ReadAll/size=32768-16 14.00 ± 0% 15.00 ± 0% +7.14% (p=0.000 n=20)
ReadAll/size=65536-16 16.00 ± 0% 16.00 ± 0% ~ (p=1.000 n=20) ¹
ReadAll/size=80000-16 17.00 ± 0% 17.00 ± 0% ~ (p=1.000 n=20) ¹
ReadAll/size=100000-16 18.00 ± 0% 17.00 ± 0% -5.56% (p=0.000 n=20)
ReadAll/size=118014-16 18.00 ± 0% 18.00 ± 0% ~ (p=1.000 n=20) ¹
ReadAll/size=131072-16 19.00 ± 0% 18.00 ± 0% -5.26% (p=0.000 n=20)
ReadAll/size=397601-16 23.00 ± 0% 22.00 ± 0% -4.35% (p=0.000 n=20)
ReadAll/size=626039-16 25.00 ± 0% 23.00 ± 0% -8.00% (p=0.000 n=20)
ReadAll/size=1000000-16 27.00 ± 0% 24.00 ± 0% -11.11% (p=0.000 n=20)
ReadAll/size=1141838-16 28.00 ± 0% 24.00 ± 0% -14.29% (p=0.000 n=20)
ReadAll/size=2414329-16 31.00 ± 0% 26.00 ± 0% -16.13% (p=0.000 n=20)
ReadAll/size=5136407-16 34.00 ± 0% 28.00 ± 0% -17.65% (p=0.000 n=20)
ReadAll/size=10000000-16 37.00 ± 0% 30.00 ± 0% -18.92% (p=0.000 n=20)
ReadAll/size=18285584-16 40.00 ± 0% 31.00 ± 0% -22.50% (p=0.000 n=20)
ReadAll/size=67379426-16 46.00 ± 0% 35.00 ± 0% -23.91% (p=0.000 n=20)
ReadAll/size=100000000-16 48.00 ± 0% 36.00 ± 0% -25.00% (p=0.000 n=20)
geomean 14.89 14.65 -1.65%
Finally, the read size in this CL currently grows exponentially
at a 1.5x growth rate. The old approach had its read size grow at
a ~1.25x growth rate once the reads are larger. We could consider
for example using a 1.25x read size growth rate here as well.
There are perhaps some ~mild trade-offs. One benefit might
be something like a ~5% smaller peak live heap contribution
(at the end, when copying into the final result) if we used
a 1.25x read growth instead of 1.5x read growth.
That said, for some systems, larger read sizes can trigger
higher throughput behavior further down the stack or
elsewhere in a system, such as via better read-ahead behavior,
larger transfer sizes, etc.
I've observed this effect in various real-world systems,
including distributed systems as well as for example with
spinning platters (which are still widely used, including
backing various "Internet scale" systems). When the effect
exists, it is usually substantial.
Therefore, my guess is it is better to get to larger read
sizes faster, which is one reason the CL is using 1.5x
read size growth rate instead of 1.25x. Also, for something
like peak live heap contribution, we are already getting
substantial wins in this CL for total heap bytes allocated,
so maybe that is OK.
(I have the actual benchmark in a separate commit, which I can
send later, or I can update this CL if preferred).
Fixes #50774
Updates #74299
Change-Id: I65eabf1d83a00fbdbe42e4c697116955f8251740
Reviewed-on: https://go-review.googlesource.com/c/go/+/722500
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
|
|
The "rt" seems to be caused after copy-pasting the previous "wt" block
which make sense as WriterTo, but for ReaderFrom it makes more sense
thinking of rf instead of rt.
Change-Id: I873699c27211bea6cdba3e199f36eb3c38188d70
GitHub-Last-Rev: 1795600a9b29946d824ba645c137da216bdf6302
GitHub-Pull-Request: golang/go#66811
Reviewed-on: https://go-review.googlesource.com/c/go/+/578635
Reviewed-by: Ian Lance Taylor <iant@google.com>
Commit-Queue: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
|
|
Change-Id: I5973a352edb73e02a274d939d6d0573788640dc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/535435
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: shuang cui <imcusg@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
|
|
Fixes #61870
Updates #61727
Change-Id: Iaef9b59c402d68f6bf64be212db2b6746abe8900
Reviewed-on: https://go-review.googlesource.com/c/go/+/526855
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
|
|
Over the past few months as I read the standard library's documentation
I kept finding spots where godoc links would have helped me.
I kept adding to a stash of changes to fix them up bit by bit.
The stash has grown big enough by now, and we're nearing a release,
so I think it's time to merge to avoid git conflicts or bit rot.
Note that a few sentences are slightly reworded,
since "implements the Fooer interface" can just be "implements [Fooer]"
now that the link provides all the context needed to the user.
Change-Id: I01c31d3d3ff066d06aeb44f545f8dd0fb9a8d998
Reviewed-on: https://go-review.googlesource.com/c/go/+/508395
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
|
|
Change-Id: Ib3e8953dbdefa2b78c31b1bcbf0909bce248e423
Reviewed-on: https://go-review.googlesource.com/c/go/+/500475
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: qiulaidongfeng <2645477756@qq.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
|
|
Fixes #40385
Change-Id: I965b5db985fd4418a992e883073cbc8309b2cb88
Reviewed-on: https://go-review.googlesource.com/c/go/+/498355
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
|
|
We don't want to permit writing before the start of an OffsetWriter.
One of the goals of OffsetWriter is to restrict where data
can be written.
However, this rule can be violated by WriteAt() method of OffsetWriter
as the following code shows:
f, _ := os.Create("file.txt")
owr := io.NewOffsetWriter(f, 10)
owr.Write([]byte("world"))
owr.WriteAt([]byte("hello"), -10)
Change-Id: I6c7519fea68daefa641f25130cdd9803dc8aae22
GitHub-Last-Rev: a29d890d6f32fd5a1ecef84d012b8447b406e2e2
GitHub-Pull-Request: golang/go#60222
Reviewed-on: https://go-review.googlesource.com/c/go/+/495155
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jabar Asadi <jasadi@d2iq.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
|
|
Refactor io.ReadAll to check for realloc of the buffer only after the
first read.
Fixes: #59702
Change-Id: I93b99139e6756f21738d47e7d9ad08e1d167258e
Reviewed-on: https://go-review.googlesource.com/c/go/+/486236
Auto-Submit: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
|
|
This reverts CL 466865.
Reason for revert: Causing trybot flakiness due to net/http race,
roll back until net/http is fixed.
For #58168
Change-Id: I3129deb996abe6466eccf933fe93cbbaf72ae217
Reviewed-on: https://go-review.googlesource.com/c/go/+/467895
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
|
|
When the race detector is enabled, scribble over copy buffers with
garbage after Write returns.
For #58452
Change-Id: I25547684bcbef7d302d76736cb02e59c89a640ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/466865
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
|
|
This reverts CL 456555.
Reason for revert: This seems too likely to exercise race conditions
in code where a Write call continues to access its buffer after
returning. The HTTP/2 ResponseWriter is one such example.
Reverting this change while we think about this some more.
For #57202
Change-Id: Ic86823f81d7da410ea6b3f17fb5b3f9a979e3340
Reviewed-on: https://go-review.googlesource.com/c/go/+/467095
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
CopyBuffer allocates a 32k buffer when no buffer is available.
Allocate these buffers from a sync.Pool.
This removes an optimization where the copy buffer size was
reduced when the source is a io.LimitedReader (including the
case of CopyN) with a limit less than the default buffer size.
This change could cause a program which only uses io.Copy
with sources with a small limit to allocate unnecessarily
large buffers. Programs which care about the transient
buffer allocation can avoid this by providing their own buffer.
name old time/op new time/op delta
CopyNSmall-10 165ns ± 7% 117ns ± 7% -29.19% (p=0.001 n=7+7)
CopyNLarge-10 7.33µs ±34% 4.07µs ± 2% -44.52% (p=0.001 n=7+7)
name old alloc/op new alloc/op delta
CopyNSmall-10 2.20kB ±12% 1.20kB ± 4% -45.24% (p=0.000 n=8+7)
CopyNLarge-10 148kB ± 9% 81kB ± 4% -45.26% (p=0.000 n=8+7)
name old allocs/op new allocs/op delta
CopyNSmall-10 2.00 ± 0% 1.00 ± 0% -50.00% (p=0.000 n=8+8)
CopyNLarge-10 2.00 ± 0% 1.00 ± 0% -50.00% (p=0.000 n=8+8)
For #57202
Change-Id: I2292226da9ba1dc09a2543f5d74fe5da06080d49
Reviewed-on: https://go-review.googlesource.com/c/go/+/456555
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Thomas Austad <thomas.austad@gmail.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
|
|
Offsetwriter refers to the design of SectionReader and removes
the section parameter n.
Since the size of the written data is determined by the user,
we cannot know where the end offset of the original data is.
The offset of SeekEnd is not valid in Seek method.
Fixes #45899.
Change-Id: I9d9445aecfa0dd4fc5168f2f65e1e3055c201b45
Reviewed-on: https://go-review.googlesource.com/c/go/+/406776
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
|
|
fixes #53474
Change-Id: I14c3dc800dc27233630a54592328bb0df1bbaa5d
GitHub-Last-Rev: 46f93cfbd41c1b3274b570744f18d08e7759eb1e
GitHub-Pull-Request: golang/go#53505
Reviewed-on: https://go-review.googlesource.com/c/go/+/413614
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
|
|
We are having a hard time deciding the exact semantics
of the Err field, and we need to ship the beta.
So revert the Err field change; it can wait for Go 1.20.
For #51115.
This reverts CL 396215.
Change-Id: I7719386567d3da10a614058a11f19dbccf304b4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/410133
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
|
|
Fixes #51115
Change-Id: I3c5296e4adc71c1c1b1808a45abd4801ae43465a
GitHub-Last-Rev: 4c197acd51e1cac051302deba57b97da66e004e1
GitHub-Pull-Request: golang/go#51990
Reviewed-on: https://go-review.googlesource.com/c/go/+/396215
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
|
|
This patch also include related fixes to net/http.
io_test.go don't test reading or WritingTo of the because the logic is simple.
NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: a6b9af4e945a6903735a74aa185e2d1c4c2e2cef
GitHub-Pull-Request: golang/go#52340
Reviewed-on: https://go-review.googlesource.com/c/go/+/400236
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
|
|
And then revert the bootstrap cmd directories and certain testdata.
And adjust tests as needed.
Not reverting the changes in std that are bootstrapped,
because some of those changes would appear in API docs,
and we want to use any consistently.
Instead, rewrite 'any' to 'interface{}' in cmd/dist for those directories
when preparing the bootstrap copy.
A few files changed as a result of running gofmt -w
not because of interface{} -> any but because they
hadn't been updated for the new //go:build lines.
Fixes #49884.
Change-Id: Ie8045cba995f65bd79c694ec77a1b3d1fe01bb09
Reviewed-on: https://go-review.googlesource.com/c/go/+/368254
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
RuneReader is fine with UTF16 or any other character encoding
Fixes #49178
Change-Id: I08a5ac205e095349d451d3b60411eaeebc3aa563
Reviewed-on: https://go-review.googlesource.com/c/go/+/359334
Trust: Meng Zhuo <mzh@golangcn.org>
Reviewed-by: Meng Zhuo <mzh@golangcn.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Meng Zhuo <mzh@golangcn.org>
TryBot-Result: Go Bot <gobot@golang.org>
|
|
implementations
Do not require the byte or rune unread by the call to match the last
return from ReadByte or ReadRune, since in practice the
implementations of these methods (especially ReadByte) may also unread
bytes from other Read-style methods without reporting an error.
Explicitly allow the Seek-like behavior implemented by bytes.Reader
and bufio.Reader, which can “unread” bytes that were never actually
read.
Explicitly allow ReadByte or ReadRune to return an error after a call
to a non-ReadByte or non-ReadRune operation respectively.
(In practice, implementations today allow very liberal calls to
ReadByte and tend to be more strict about ReadRune, but it seems
simpler to keep the two definitions completely parallel.)
Like CL 349054, this is techincally a breaking change, but given the
long-standing behavior of the implementations in the Go standard
library (such as strings.Reader, bytes.Buffer, and bufio.Reader),
I believe it falls under the “specification errors” exception to the
Go 1 compatibility policy.
Fixes #48449
Change-Id: I61696a59770fe83c667377ba25a072762d3f6f19
Reviewed-on: https://go-review.googlesource.com/c/go/+/351809
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
|
|
Fixes #48620
Change-Id: I37a5909ad27dc4a170929cb2e2ed1045cf524d59
Reviewed-on: https://go-review.googlesource.com/c/go/+/352629
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
|
|
Change-Id: Ie23a9f1300a803d9c713e82b0d892dd90333ca7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/351371
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Use “or” instead of “and” to describe error behavior.
On error, nearly all Seeker implementations in the Go repo return
0 instead of “the new offset”. (Arguably on error “the new offset”
is the same as the previous offset, but some Seeker implementations
don't have that offset readily available.)
Don't claim that “any positive offsite is legal”.
In practice, most of the Seeker implementations within the Go standard
library do not allow “[s]eeking to any [arbitrary] positive offset”:
some reject all out-of-bounds offsets, and some reject only a subset
that happen to overflow some underlying representation. Since some
positive offsets may be rejected, we cannot claim that seeking to
those offsets “is legal”. However, to avoid invalidating existing
Seeker implemetations we must not require an implementation to reject
invalid positive offsets either.
This is technically a breaking change, since callers of Seek are no
longer allowed to assume that a Seek resulting in an arbitrary
positive offset will succeed. However, since basically none of the
existing implementations actually conformed to the documented behavior
I believe this falls under the “specification errors” exception to the
Go 1 compatibility policy.
Fixes #48316
Change-Id: Ib1b478599b20ad5361bcc97fe8ceb84f74e6d971
Reviewed-on: https://go-review.googlesource.com/c/go/+/349054
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
|
|
In the process of refactoring ioutil.Discard to io.Discard in
CL 263141 "an" should have been changed to "a" but was likely
missed in the process.
This commit corrects the spelling of the documentation.
Change-Id: I0609c45878291f8f01560efc3f3e6fba191e095b
GitHub-Last-Rev: e3257ca272dff42ed7d07b6e6a5fc49493772653
GitHub-Pull-Request: golang/go#45190
Reviewed-on: https://go-review.googlesource.com/c/go/+/304209
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Griesemer <gri@golang.org>
|
|
The old ioutil references are still valid, but update our code
to reflect best practices and get used to the new locations.
Code compiled with the bootstrap toolchain
(cmd/asm, cmd/dist, cmd/compile, debug/elf)
must remain Go 1.4-compatible and is excluded.
Also excluded vendored code.
For #41190.
Change-Id: I6d86f2bf7bc37a9d904b6cee3fe0c7af6d94d5b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/263142
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
|
|
As proposed and approved in #40025, Discard, NopCloser, and ReadAll
do not really fit into io/ioutil, which exists mainly to hold things that
would cause an import cycle if implemented in io itself, which is to say
things that import "os".
These three do not import "os" - they are generic io helpers like
many of the things in io itself, so it makes sense for them to be there.
Fixes #40025.
Change-Id: I77f47e9b2a72839edf7446997936631980047b67
Reviewed-on: https://go-review.googlesource.com/c/go/+/263141
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
|
|
It was added in CL 240740 to fix #39978
but without any discussion of the exported API.
The error can still be returned to fix the issue,
without adding new public API to package io.
Also fix the error message to refer to lower-case write
like the other errors in the package.
Change-Id: I134de5eaf3ac903d73913c5cadcde904c5255d79
Reviewed-on: https://go-review.googlesource.com/c/go/+/262877
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
|
|
Research showed that this interface is defined frequently enough in
real-world usage to justify its addition to the standard library.
Fixes #40962
Change-Id: I522fe8f9b8753c3fa42ccc1def49611cf88cd340
GitHub-Last-Rev: 6a45be66b42e482a06d9809d9da20c195380988b
GitHub-Pull-Request: golang/go#41939
Reviewed-on: https://go-review.googlesource.com/c/go/+/261577
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
|
|
Fixes #39978
Change-Id: Ib41459861ba9f7cf0bf1fc95b1479c358c4bdbd8
GitHub-Last-Rev: 19cbb1461ca04a8eb64f0c4f354d8fb81a70d4f3
GitHub-Pull-Request: golang/go#39989
Reviewed-on: https://go-review.googlesource.com/c/go/+/240740
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
|
|
For #40827.
Change-Id: Ifd108421abd8d0988dd7b985e4f9e2bd5356964a
Reviewed-on: https://go-review.googlesource.com/c/go/+/258524
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
|
|
Offered as an alternative to CL 221380, which was more
tutorial than necessary.
Update #37344
Change-Id: Ide673b0b97983c2c2319a9311dc3d0a10567e6c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/223097
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Document that if either src implements the WriteTo interface
or if dst implements the ReaderFrom interface, then
buf will not be used.
Fixes #32276
Change-Id: Id0a69c90e255e694e7ec9f79ffe4d8391441e59e
GitHub-Last-Rev: 750e7e86d5d9b985fae7f2329fd219cacf72a62b
GitHub-Pull-Request: golang/go#32279
Reviewed-on: https://go-review.googlesource.com/c/go/+/179137
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
|
|
And start using it elsewhere in the standard library, removing the
copies in the process.
While at it, rewrite the io.WriteString godoc to be more clear, since it
can now make reference to the defined interface.
Fixes #27946.
Change-Id: Id5ba223c09c19e5fb49815bd3b1bd3254fc786f3
Reviewed-on: https://go-review.googlesource.com/c/139457
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Change-Id: I930be9027fb972198b3d44816a5e4f53ff7eb5ea
Reviewed-on: https://go-review.googlesource.com/111642
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Add a note that if an error is returned after having read
at least the minimum no. of bytes, the error is set to nil.
Fixes #20477
Change-Id: I75ba5ee967be3ff80249e40d459da4afeeb53463
Reviewed-on: https://go-review.googlesource.com/102459
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
CL 60630 claimed to and did “improve performance of CopyN”
but in doing so introduced a second copy of the I/O copying loop.
This code is subtle and easy to get wrong and the last thing we
need is of two copies that can drift out of sync. Even the newly
introduced copy contains various subtle changes that are not
obviously semantically equivalent to the original. (They probably
are, but it's not obvious.)
Although the CL description does not explain further what the
important optimization was, it appears that the most critical
one was not allocating a 32kB buffer for CopyN(w, r, 512).
This CL deletes the forked copy of copy and instead applies
the buffer size restriction optimization directly to copy itself.
CL 60630 reported:
name old time/op new time/op delta
CopyNSmall-4 5.09µs ± 1% 2.25µs ±86% -55.91% (p=0.000 n=11+14)
CopyNLarge-4 114µs ±73% 121µs ±72% ~ (p=0.701 n=14+14)
Starting with that CL as the baseline, this CL does not change a ton:
name old time/op new time/op delta
CopyNSmall-8 370ns ± 1% 411ns ± 1% +11.18% (p=0.000 n=16+14)
CopyNLarge-8 18.2µs ± 1% 18.3µs ± 1% +0.63% (p=0.000 n=19+20)
It does give up a small amount of the win of 60630 but preserves
the bulk of it, with the benefit that we will not need to debug these
two copies drifting out of sync in the future.
Change-Id: I05b1a5a7115390c5867847cba606b75d513eb2e2
Reviewed-on: https://go-review.googlesource.com/78122
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
|
|
Benchmarks:
name old time/op new time/op delta
CopyNSmall-4 5.09µs ± 1% 2.25µs ±86% -55.91% (p=0.000 n=11+14)
CopyNLarge-4 114µs ±73% 121µs ±72% ~ (p=0.701 n=14+14)
name old alloc/op new alloc/op delta
CopyNSmall-4 34.6kB ± 0% 1.9kB ±19% -94.60% (p=0.000 n=12+14)
CopyNLarge-4 129kB ± 8% 127kB ±18% -2.00% (p=0.007 n=14+14)
name old allocs/op new allocs/op delta
CopyNSmall-4 2.00 ± 0% 1.00 ± 0% -50.00% (p=0.000 n=14+14)
CopyNLarge-4 2.00 ± 0% 1.00 ± 0% -50.00% (p=0.000 n=14+14)
Benchmark code:
type Buffer struct {
bytes.Buffer
io.ReaderFrom
}
func BenchmarkCopyNSmall(b *testing.B) {
bs := bytes.Repeat([]byte{0}, 1024)
rd := bytes.NewReader(bs)
buf := new(Buffer)
b.ResetTimer()
for i := 0; i < b.N; i++ {
io.CopyN(buf, rd, 512)
rd.Reset(bs)
}
}
func BenchmarkCopyNLarge(b *testing.B) {
bs := bytes.Repeat([]byte{0}, 64*1024)
rd := bytes.NewReader(bs)
buf := new(Buffer)
b.ResetTimer()
for i := 0; i < b.N; i++ {
io.CopyN(buf, rd, (32*1024)+1)
rd.Reset(bs)
}
}
Change-Id: Id8d29e55758452c870cf372db640f07baec05849
Reviewed-on: https://go-review.googlesource.com/60630
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Document that the byte value returned by ReadByte() is meaningless
if its error != nil. Because io.Reader and io.ByteReader are similar in
name, this CL aims to clear up any ambiguity surrounding the returned
values, particularly where io.Reader is allowed to return both a
non-zero number of valid bytes and err == EOF.
Fixes #20825
Change-Id: I3a23c18c80c471c0caae3b4d2f6f8e547da0bed9
Reviewed-on: https://go-review.googlesource.com/46950
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Specify that that LimitedReader returns EOF when the underlying
R returns EOF even if bytes remaining, N > 0.
Fixes #18271
Change-Id: I990a7135f1d31488d535238ae061d42ee96bacb7
Reviewed-on: https://go-review.googlesource.com/34249
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Change-Id: I0b7052103174f0864ee9714f76f8f78f2a988777
Reviewed-on: https://go-review.googlesource.com/30719
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
The documentation previously used C style enumerations: 0, 1, 2.
While this is pretty much universally correct, it does not help a user
become aware of the existence of the SeekStart, SeekCurrent, and SeekEnd
constants. Thus, we should use them in the documentation to direct people's
attention to them.
Updates #6885
Change-Id: I44b5e78d41601c68a0a1c96428c853df53981d52
Reviewed-on: https://go-review.googlesource.com/23551
Reviewed-by: Andrew Gerrand <adg@golang.org>
|
|
It's not clear we want to enshrine an io interface in which Size cannot
return an error. Because this requires more thought before committing
to the API, remove from Go 1.7.
Fixes #15818.
Change-Id: Ic4138ffb0e033030145a12d33f78078350a8381f
Reviewed-on: https://go-review.googlesource.com/23392
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
|
|
CL/19862 (f79b50b8d5bc159561c1dcf7c17e2a0db96a9a11) recently introduced the constants
SeekStart, SeekCurrent, and SeekEnd to the io package. We should use these constants
consistently throughout the code base.
Updates #15269
Change-Id: If7fcaca7676e4a51f588528f5ced28220d9639a2
Reviewed-on: https://go-review.googlesource.com/22097
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Joe Tsai <joetsai@digital-static.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Fixes #13849
Change-Id: Idd7f06b547a0179fe15571807a8c48b7c3b78d7c
Reviewed-on: https://go-review.googlesource.com/21852
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
This is a proposal. The old name is pretty poor. The new one describes
it better and may be easier to remember. It does not start with Read,
though I think that inconsistency is worthwhile.
Reworded the comment a bit for clarity.
Change-Id: Icb4f9c663cc68958e0363d7ff78a0b29cc521f98
Reviewed-on: https://go-review.googlesource.com/21629
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
ReadAtSizer is a common abstraction for a stateless,
concurrently-readable fixed number of bytes.
This interface has existed in various codebases for over 3 years (previously
usually named SizeReaderAt). It is used inside Google in dl.google.com
(mentioned in https://talks.golang.org/2013/oscon-dl.slide) and other
packages. It is used in Camlistore, in Juju, in the Google API Go client, in
github.com/nightlyone/views, and 33 other pages of Github search results.
It is implemented by io.SectionReader, bytes.Reader, strings.Reader, etc.
Time to finally promote this interface to the standard library and give it a
standard name, blessing it as best practice.
Updates #7263
Updates #14889
Change-Id: Id28c0cafa7d2d37e8887c54708b5daf1b11c83ea
Reviewed-on: https://go-review.googlesource.com/21492
Reviewed-by: Rob Pike <r@golang.org>
|
|
Fixes #6885
Change-Id: I6907958186f6a2427da1ad2f6c20bd5d7bf7a3f9
Reviewed-on: https://go-review.googlesource.com/19862
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
The tree's pretty inconsistent about single space vs double space
after a period in documentation. Make it consistently a single space,
per earlier decisions. This means contributors won't be confused by
misleading precedence.
This CL doesn't use go/doc to parse. It only addresses // comments.
It was generated with:
$ perl -i -npe 's,^(\s*// .+[a-z]\.) +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.) +([A-Z])')
$ go test go/doc -update
Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7
Reviewed-on: https://go-review.googlesource.com/20022
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Named returned values should only be used on public funcs and methods
when it contributes to the documentation.
Named return values should not be used if they're only saving the
programmer a few lines of code inside the body of the function,
especially if that means there's stutter in the documentation or it
was only there so the programmer could use a naked return
statement. (Naked returns should not be used except in very small
functions)
This change is a manual audit & cleanup of public func signatures.
Signatures were not changed if:
* the func was private (wouldn't be in public godoc)
* the documentation referenced it
* the named return value was an interesting name. (i.e. it wasn't
simply stutter, repeating the name of the type)
There should be no changes in behavior. (At least: none intended)
Change-Id: I3472ef49619678fe786e5e0994bdf2d9de76d109
Reviewed-on: https://go-review.googlesource.com/20024
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
|
