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

Jump to bottom

strings: TestCompareStrings times out on the linux-arm* builders #26155

Closed
mundaym opened this issue Jun 30, 2018 · 10 comments
Closed

strings: TestCompareStrings times out on the linux-arm* builders #26155

mundaym opened this issue Jun 30, 2018 · 10 comments
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Go1.12

Comments

@mundaym
Copy link
Member

mundaym commented Jun 30, 2018

This test was extended in CL 121495 and is now timing out on the linux-arm* builders. We could avoid running the longer test on the arm builder or we could look into optimizing the test a bit (it currently does quite a bit of repeated work).

/cc @bradfitz @billotosyr
@mundaym mundaym added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Jun 30, 2018
@mundaym mundaym added this to the Go1.11 milestone Jun 30, 2018
@mvdan
Copy link
Member

mvdan commented Jun 30, 2018

Perhaps a simpler short-term solution is to skip it in short mode.

@ianlancetaylor ianlancetaylor mentioned this issue Jul 1, 2018
Closed
@ianlancetaylor
Copy link
Contributor

@mvdan The longer parts of the tests are currently skipped in short mode, but not on the builders.

@ianlancetaylor
Copy link
Contributor

On my laptop the test does 23581 garbage collections.

@gopherbot
Copy link

Change https://golang.org/cl/121820 mentions this issue: strings: do much less redundant testing in TestCompareStrings

@aclements
Copy link
Member

This also is probably causing timeouts on openbsd-amd64-62.

gopherbot pushed a commit that referenced this issue Jul 2, 2018
@ianlancetaylor
strings: do much less redundant testing in TestCompareStrings
869884d 
On the OpenBSD builder this reduces the test time from 213 seconds to
60 seconds, without loss of testing.

Not sure why the test is so much slower on OpenBSD, so not closing the
issues.

Updates #26155
Updates #26174

Change-Id: I13b58bbe3b209e591c308765077d2342943a3d2a
Reviewed-on: https://go-review.googlesource.com/121820
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ralph Corderoy <ralph@inputplus.co.uk>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@ianlancetaylor
Copy link
Contributor

The test should now pass, so removing the release-blocker label and changing this into an investigation of why the test is so much slower on ARM than on amd64.

@ianlancetaylor ianlancetaylor added Performance help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Jul 2, 2018
@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jul 2, 2018
@millerresearch
Copy link
Contributor

FYI, before the CL, the test was timing out on plan9_arm too.

@gopherbot
Copy link

Change https://golang.org/cl/124115 mentions this issue: [release-branch.go1.10] strings: do much less redundant testing in TestCompareStrings

gopherbot pushed a commit that referenced this issue Jul 16, 2018
@ianlancetaylor
[release-branch.go1.10] strings: do much less redundant testing in Te…
4c77cb3 
…stCompareStrings

On the OpenBSD builder this reduces the test time from 213 seconds to
60 seconds, without loss of testing.

Not sure why the test is so much slower on OpenBSD, so not closing the
issues.

Updates #26155
Updates #26174

Change-Id: I13b58bbe3b209e591c308765077d2342943a3d2a
Reviewed-on: https://go-review.googlesource.com/121820
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ralph Corderoy <ralph@inputplus.co.uk>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 869884d)
Reviewed-on: https://go-review.googlesource.com/124115
Reviewed-by: Bill O'Farrell <billotosyr@gmail.com>
@gopherbot
Copy link

Change https://golang.org/cl/124295 mentions this issue: [release-branch.go1.9] strings: do much less redundant testing in TestCompareStrings

@gopherbot
Copy link

Change https://golang.org/cl/147358 mentions this issue: strings: lower running time of TestCompareStrings

@golang golang locked and limited conversation to collaborators Nov 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
6 participants
@aclements @ianlancetaylor @mvdan @gopherbot @mundaym @millerresearch