[go] io: ReadFile: don't check for slice re-allocation in the first iteration

9 views
Skip to first unread message

Gerrit Bot (Gerrit)

unread,
May 27, 2023, 3:57:19 PM5/27/23
to goph...@pubsubhelper.golang.org, Jabar Asadi, golang-co...@googlegroups.com

Gerrit Bot has uploaded this change for review.

View Change

io: ReadFile: don't check for slice re-allocation in the first iteration

At the beginning of the for-loop iteration size > 0 always. so in
the first iteration, this check is not necessary. we can move this check to
after the read operation.

besides this change >= operator also can be simplified to == in the if-statement

Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
GitHub-Last-Rev: b53bfd551cf7e57f12285e50bab083d349144a63
GitHub-Pull-Request: golang/go#60473
---
M src/os/file.go
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/os/file.go b/src/os/file.go
index 7fd2f5d..4b9f1df 100644
--- a/src/os/file.go
+++ b/src/os/file.go
@@ -737,10 +737,6 @@

data := make([]byte, 0, size)
for {
- if len(data) >= cap(data) {
- d := append(data[:cap(data)], 0)
- data = d[:len(data)]
- }
n, err := f.Read(data[len(data):cap(data)])
data = data[:len(data)+n]
if err != nil {
@@ -749,6 +745,11 @@
}
return data, err
}
+
+ if len(data) == cap(data) {
+ d := append(data[:cap(data)], 0)
+ data = d[:len(data)]
+ }
}
}


To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: newchange
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
Gerrit-Change-Number: 498915
Gerrit-PatchSet: 1
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-CC: Jabar Asadi <jas...@d2iq.com>

Rob Pike (Gerrit)

unread,
May 27, 2023, 7:14:40 PM5/27/23
to Gerrit Bot, Jabar Asadi, goph...@pubsubhelper.golang.org, Ian Lance Taylor, Rob Pike, Russ Cox, Gopher Robot, golang-co...@googlegroups.com

Attention is currently required from: Ian Lance Taylor, Russ Cox.

View Change

1 comment:

  • File src/os/file.go:

    • Patch Set #1, Line 749: if len(data) == cap(data) {

      yes, == is correct here, but i prefer the defensive programming of >=. after too many years programming with corrupted data and other bugs. i'd rather you left it as is, because the existing code is not wrong. there is no need to change it.

      i may be the only person who feels this way, though.

To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
Gerrit-Change-Number: 498915
Gerrit-PatchSet: 1
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Rob Pike <r...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-CC: Jabar Asadi <jas...@d2iq.com>
Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
Gerrit-Attention: Russ Cox <r...@golang.org>
Gerrit-Comment-Date: Sat, 27 May 2023 23:14:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

Jabar Asadi (Gerrit)

unread,
May 28, 2023, 2:24:27 AM5/28/23
to Gerrit Bot, goph...@pubsubhelper.golang.org, Ian Lance Taylor, Rob Pike, Russ Cox, Gopher Robot, golang-co...@googlegroups.com

Attention is currently required from: Ian Lance Taylor, Rob Pike, Russ Cox.

View Change

1 comment:

  • File src/os/file.go:

    • yes, == is correct here, but i prefer the defensive programming of >=. […]

      sure, Done.

To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
Gerrit-Change-Number: 498915
Gerrit-PatchSet: 1
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Rob Pike <r...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-CC: Jabar Asadi <jas...@d2iq.com>
Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
Gerrit-Attention: Rob Pike <r...@golang.org>
Gerrit-Attention: Russ Cox <r...@golang.org>
Gerrit-Comment-Date: Sun, 28 May 2023 06:24:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Rob Pike <r...@golang.org>

Gerrit Bot (Gerrit)

unread,
May 28, 2023, 2:24:28 AM5/28/23
to Jabar Asadi, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Ian Lance Taylor, Rob Pike, Russ Cox.

Gerrit Bot uploaded patch set #2 to this change.

View Change

io: ReadFile: don't check for slice re-allocation in the first iteration

At the beginning of the for-loop iteration size > 0 always. so in
the first iteration, this check is not necessary. we can move this check to
after the read operation.

besides this change >= operator also can be simplified to == in the if-statement

Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
GitHub-Last-Rev: 1c568d229551eca3a7b6fbb51b76ec6cc7e8e899

GitHub-Pull-Request: golang/go#60473
---
M src/os/file.go
1 file changed, 5 insertions(+), 4 deletions(-)

To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: newpatchset
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
Gerrit-Change-Number: 498915
Gerrit-PatchSet: 2

Gerrit Bot (Gerrit)

unread,
May 28, 2023, 2:29:56 AM5/28/23
to Jabar Asadi, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Ian Lance Taylor, Rob Pike, Russ Cox.

Gerrit Bot uploaded patch set #3 to this change.

View Change

os: ReadFile: don't check for re-allocation in the first iteration


At the beginning of the for-loop iteration size > 0 always. so in
the first iteration, this check is not necessary. we can move this check to
after the read operation.

besides this change >= operator also can be simplified to == in the if-statement

Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
GitHub-Last-Rev: 2fdf25dff2e9984d3a8f8e5e612ea802c88e88a1

GitHub-Pull-Request: golang/go#60473
---
M src/os/file.go
1 file changed, 5 insertions(+), 4 deletions(-)

To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: newpatchset
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
Gerrit-Change-Number: 498915
Gerrit-PatchSet: 3

Gerrit Bot (Gerrit)

unread,
May 28, 2023, 2:45:27 AM5/28/23
to Jabar Asadi, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Ian Lance Taylor, Rob Pike, Russ Cox.

Gerrit Bot uploaded patch set #4 to this change.

View Change

os: ReadFile: don't check for re-allocation in the first iteration

At the beginning of the for-loop iteration cap(data) > len(data) always.
Therefore, in the first iteration, this check becomes unnecessary.

we can move this check to after the read operation.

Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
GitHub-Last-Rev: 2fdf25dff2e9984d3a8f8e5e612ea802c88e88a1
GitHub-Pull-Request: golang/go#60473
---
M src/os/file.go
1 file changed, 5 insertions(+), 4 deletions(-)

To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: newpatchset
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
Gerrit-Change-Number: 498915
Gerrit-PatchSet: 4

Rob Pike (Gerrit)

unread,
May 28, 2023, 3:40:57 AM5/28/23
to Gerrit Bot, Jabar Asadi, goph...@pubsubhelper.golang.org, Rob Pike, Ian Lance Taylor, Russ Cox, Gopher Robot, golang-co...@googlegroups.com

Attention is currently required from: Ian Lance Taylor, Russ Cox.

Patch set 4:Run-TryBot +1

View Change

    To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
    Gerrit-Change-Number: 498915
    Gerrit-PatchSet: 4
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Reviewer: Rob Pike <r...@golang.org>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: Jabar Asadi <jas...@d2iq.com>
    Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Comment-Date: Sun, 28 May 2023 07:40:52 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes

    Ian Lance Taylor (Gerrit)

    unread,
    May 29, 2023, 7:13:55 PM5/29/23
    to Gerrit Bot, Jabar Asadi, goph...@pubsubhelper.golang.org, Gopher Robot, Rob Pike, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com

    Attention is currently required from: Ian Lance Taylor, Russ Cox.

    Patch set 4:Run-TryBot +1Auto-Submit +1Code-Review +2

    View Change

    1 comment:

    To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
    Gerrit-Change-Number: 498915
    Gerrit-PatchSet: 4
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Gopher Robot <go...@golang.org>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@google.com>
    Gerrit-Reviewer: Rob Pike <r...@golang.org>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Jabar Asadi <jas...@d2iq.com>
    Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Comment-Date: Mon, 29 May 2023 23:13:52 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes

    Ian Lance Taylor (Gerrit)

    unread,
    May 29, 2023, 7:18:54 PM5/29/23
    to Gerrit Bot, Jabar Asadi, goph...@pubsubhelper.golang.org, Gopher Robot, Rob Pike, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com

    Attention is currently required from: Ian Lance Taylor, Russ Cox.

    Patch set 4:-Auto-Submit-Code-Review

    View Change

    1 comment:

    • Patchset:

    To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
    Gerrit-Change-Number: 498915
    Gerrit-PatchSet: 4
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Gopher Robot <go...@golang.org>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@google.com>
    Gerrit-Reviewer: Rob Pike <r...@golang.org>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Jabar Asadi <jas...@d2iq.com>
    Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Comment-Date: Mon, 29 May 2023 23:18:50 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes

    Jabar Asadi (Gerrit)

    unread,
    May 30, 2023, 5:07:33 AM5/30/23
    to Gerrit Bot, goph...@pubsubhelper.golang.org, Gopher Robot, Rob Pike, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com

    Attention is currently required from: Ian Lance Taylor, Ian Lance Taylor, Russ Cox.

    View Change

    1 comment:

    • Patchset:

      • Thanks!

    To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
    Gerrit-Change-Number: 498915
    Gerrit-PatchSet: 4
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Gopher Robot <go...@golang.org>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@google.com>
    Gerrit-Reviewer: Rob Pike <r...@golang.org>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Jabar Asadi <jas...@d2iq.com>
    Gerrit-Attention: Ian Lance Taylor <ia...@google.com>
    Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Comment-Date: Tue, 30 May 2023 09:07:28 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Ian Lance Taylor <ia...@google.com>

    Rob Pike (Gerrit)

    unread,
    Jul 24, 2023, 6:25:30 AM7/24/23
    to Gerrit Bot, Jabar Asadi, goph...@pubsubhelper.golang.org, Rob Pike, Gopher Robot, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com

    Attention is currently required from: Ian Lance Taylor, Ian Lance Taylor, Russ Cox.

    Patch set 4:Code-Review +2

    View Change

    1 comment:

    • Patchset:

      • Patch Set #4:

        Not your fault, as you're just rearranging existing pieces, but that loop is too tricky for my taste.

        But that's an issue for another day.

    To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
    Gerrit-Change-Number: 498915
    Gerrit-PatchSet: 4
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Gopher Robot <go...@golang.org>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@google.com>
    Gerrit-Reviewer: Rob Pike <r...@golang.org>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Jabar Asadi <jas...@d2iq.com>
    Gerrit-Attention: Ian Lance Taylor <ia...@google.com>
    Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Comment-Date: Mon, 24 Jul 2023 10:25:23 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes

    Bryan Mills (Gerrit)

    unread,
    Jul 24, 2023, 3:29:29 PM7/24/23
    to Gerrit Bot, Jabar Asadi, goph...@pubsubhelper.golang.org, Bryan Mills, Rob Pike, Gopher Robot, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com

    Attention is currently required from: Ian Lance Taylor, Ian Lance Taylor, Russ Cox.

    Patch set 4:Code-Review +1

    View Change

      To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: comment
      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
      Gerrit-Change-Number: 498915
      Gerrit-PatchSet: 4
      Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
      Gerrit-Reviewer: Bryan Mills <bcm...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
      Gerrit-Reviewer: Ian Lance Taylor <ia...@google.com>
      Gerrit-Reviewer: Rob Pike <r...@golang.org>
      Gerrit-Reviewer: Russ Cox <r...@golang.org>
      Gerrit-CC: Jabar Asadi <jas...@d2iq.com>
      Gerrit-Attention: Ian Lance Taylor <ia...@google.com>
      Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
      Gerrit-Attention: Russ Cox <r...@golang.org>
      Gerrit-Comment-Date: Mon, 24 Jul 2023 19:29:25 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes

      Ian Lance Taylor (Gerrit)

      unread,
      Jul 24, 2023, 8:47:21 PM7/24/23
      to Gerrit Bot, Jabar Asadi, goph...@pubsubhelper.golang.org, Bryan Mills, Rob Pike, Gopher Robot, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com

      Attention is currently required from: Ian Lance Taylor, Russ Cox.

      Patch set 4:Auto-Submit +1Code-Review +2

      View Change

      1 comment:

      To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: comment
      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
      Gerrit-Change-Number: 498915
      Gerrit-PatchSet: 4
      Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
      Gerrit-Reviewer: Bryan Mills <bcm...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
      Gerrit-Reviewer: Ian Lance Taylor <ia...@google.com>
      Gerrit-Reviewer: Rob Pike <r...@golang.org>
      Gerrit-Reviewer: Russ Cox <r...@golang.org>
      Gerrit-CC: Jabar Asadi <jas...@d2iq.com>
      Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
      Gerrit-Attention: Russ Cox <r...@golang.org>
      Gerrit-Comment-Date: Tue, 25 Jul 2023 00:47:16 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes

      Gopher Robot (Gerrit)

      unread,
      Jul 24, 2023, 8:48:11 PM7/24/23
      to Gerrit Bot, Jabar Asadi, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Bryan Mills, Rob Pike, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com

      Gopher Robot submitted this change.

      View Change

      Approvals: Ian Lance Taylor: Looks good to me, approved; Run TryBots; Automatically submit change Rob Pike: Looks good to me, approved; Run TryBots Gopher Robot: TryBots succeeded Bryan Mills: Looks good to me, but someone else must approve
      os: ReadFile: don't check for re-allocation in the first iteration

      At the beginning of the for-loop iteration cap(data) > len(data) always.
      Therefore, in the first iteration, this check becomes unnecessary.
      we can move this check to after the read operation.

      Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
      GitHub-Last-Rev: 2fdf25dff2e9984d3a8f8e5e612ea802c88e88a1
      GitHub-Pull-Request: golang/go#60473
      Reviewed-on: https://go-review.googlesource.com/c/go/+/498915
      Reviewed-by: Rob Pike <r...@golang.org>
      Reviewed-by: Bryan Mills <bcm...@google.com>
      Run-TryBot: Rob Pike <r...@golang.org>
      TryBot-Result: Gopher Robot <go...@golang.org>
      Run-TryBot: Ian Lance Taylor <ia...@google.com>
      Reviewed-by: Ian Lance Taylor <ia...@google.com>
      Auto-Submit: Ian Lance Taylor <ia...@google.com>

      ---
      M src/os/file.go
      1 file changed, 5 insertions(+), 4 deletions(-)

      
      
      diff --git a/src/os/file.go b/src/os/file.go
      index 806c1f2..2f12c3b 100644
      --- a/src/os/file.go
      +++ b/src/os/file.go
      @@ -725,10 +725,6 @@


      data := make([]byte, 0, size)
      for {
      - if len(data) >= cap(data) {
      - d := append(data[:cap(data)], 0)
      - data = d[:len(data)]
      - }
      n, err := f.Read(data[len(data):cap(data)])
      data = data[:len(data)+n]
      if err != nil {
      @@ -737,6 +733,11 @@
      }
      return data, err
      }
      +
      + if len(data) >= cap(data) {

      + d := append(data[:cap(data)], 0)
      + data = d[:len(data)]
      + }
      }
      }


      To view, visit change 498915. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: merged
      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
      Gerrit-Change-Number: 498915
      Gerrit-PatchSet: 5
      Reply all
      Reply to author
      Forward
      0 new messages