[tools] gopls/benchmark: skip unimported completion if not local

3 views
Skip to first unread message

Peter Weinberger (Gerrit)

unread,
Dec 22, 2025, 4:11:11 PM (2 days ago) Dec 22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Peter Weinberger has uploaded the change for review

Commit message

gopls/benchmark: skip unimported completion if not local

This benchmark should only be run locally, where there is a usable
module cache (and index). Also, it had an obsolete calculation of
the location of the index.

For context, see
 https://github.com/golang/go/issues/76873#issuecomment-3667491422
Change-Id: I5226671d789d38d67a40628d6886293d00f659fc

Change diff

diff --git a/gopls/internal/test/integration/bench/unimported_test.go b/gopls/internal/test/integration/bench/unimported_test.go
index baa1d3c..8d4d945 100644
--- a/gopls/internal/test/integration/bench/unimported_test.go
+++ b/gopls/internal/test/integration/bench/unimported_test.go
@@ -22,6 +22,7 @@
 // experiments show the new code about 15 times faster than the old,
 // and the old code sometimes fails to find the completion
 func BenchmarkLocalModcache(b *testing.B) {
+	b.Skip("only run by hand")
 	budgets := []string{"0s", "100ms", "200ms", "500ms", "1s", "5s"}
 	sources := []string{"gopls", "goimports"}
 	for _, budget := range budgets {
@@ -99,7 +100,7 @@
 	// Check that completion works as expected
 	env.CreateBuffer("main.go", mainfile)
 	env.AfterChange()
-	if false { // warm up? or not?
+	if true { // with true, reveals failutes
 		loc := env.RegexpSearch("main.go", name)
 		completions := env.Completion(loc)
 		if len(completions.Items) == 0 {
@@ -153,6 +154,6 @@
 	if err != nil {
 		t.Fatalf("os.UserCacheDir: %v", err)
 	}
-	dir = filepath.Join(dir, "go", "imports")
+	dir = filepath.Join(dir, "goimports")
 	modindex.IndexDir = dir
 }
diff --git a/internal/modindex/index.go b/internal/modindex/index.go
index c41d1dd..94bae4a 100644
--- a/internal/modindex/index.go
+++ b/internal/modindex/index.go
@@ -107,7 +107,7 @@
 		var err error
 		dir, err = os.UserCacheDir()
 		// shouldn't happen, but TempDir is better than
-		// creating ./go/imports
+		// creating ./goimports
 		if err != nil {
 			dir = os.TempDir()
 		}

Change information

Files:
  • M gopls/internal/test/integration/bench/unimported_test.go
  • M internal/modindex/index.go
Change size: XS
Delta: 2 files changed, 4 insertions(+), 3 deletions(-)
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: tools
Gerrit-Branch: master
Gerrit-Change-Id: I5226671d789d38d67a40628d6886293d00f659fc
Gerrit-Change-Number: 732000
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Weinberger <p...@google.com>
Gerrit-Reviewer: Peter Weinberger <p...@google.com>
unsatisfied_requirement
satisfied_requirement
open
diffy

Alan Donovan (Gerrit)

unread,
Dec 22, 2025, 5:32:35 PM (2 days ago) Dec 22
to Peter Weinberger, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
Attention needed from Peter Weinberger

Alan Donovan added 2 comments

File gopls/internal/test/integration/bench/unimported_test.go
Line 25, Patchset 1 (Latest): b.Skip("only run by hand")
Alan Donovan . unresolved

Let's be specific about why this can only be run "by hand", or more accurately, locally after some sequence of setup steps documented somewhere.

Line 103, Patchset 1 (Latest): if true { // with true, reveals failutes
Alan Donovan . unresolved

Simplify? (Why would one not want to see failures?)

(BTW: typo)

Open in Gerrit

Related details

Attention is currently required from:
  • Peter Weinberger
Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: tools
    Gerrit-Branch: master
    Gerrit-Change-Id: I5226671d789d38d67a40628d6886293d00f659fc
    Gerrit-Change-Number: 732000
    Gerrit-PatchSet: 1
    Gerrit-Owner: Peter Weinberger <p...@google.com>
    Gerrit-Reviewer: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Peter Weinberger <p...@google.com>
    Gerrit-Attention: Peter Weinberger <p...@google.com>
    Gerrit-Comment-Date: Mon, 22 Dec 2025 22:32:32 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Peter Weinberger (Gerrit)

    unread,
    Dec 23, 2025, 9:58:06 AM (yesterday) Dec 23
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
    Attention needed from Peter Weinberger

    Peter Weinberger uploaded new patchset

    Peter Weinberger uploaded patch set #2 to this change.
    Following approvals got outdated and were removed:
    • TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Peter Weinberger
    Submit Requirements:
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      • requirement is not satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: newpatchset
      Gerrit-Project: tools
      Gerrit-Branch: master
      Gerrit-Change-Id: I5226671d789d38d67a40628d6886293d00f659fc
      Gerrit-Change-Number: 732000
      Gerrit-PatchSet: 2
      unsatisfied_requirement
      open
      diffy

      Peter Weinberger (Gerrit)

      unread,
      Dec 23, 2025, 10:58:20 AM (yesterday) Dec 23
      to goph...@pubsubhelper.golang.org, Go LUCI, Alan Donovan, golang-co...@googlegroups.com
      Attention needed from Alan Donovan

      Peter Weinberger added 2 comments

      File gopls/internal/test/integration/bench/unimported_test.go
      Line 25, Patchset 1: b.Skip("only run by hand")
      Alan Donovan . resolved

      Let's be specific about why this can only be run "by hand", or more accurately, locally after some sequence of setup steps documented somewhere.

      Peter Weinberger

      Done

      Line 103, Patchset 1: if true { // with true, reveals failutes
      Alan Donovan . resolved

      Simplify? (Why would one not want to see failures?)

      (BTW: typo)

      Peter Weinberger

      Done

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Alan Donovan
      Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: tools
        Gerrit-Branch: master
        Gerrit-Change-Id: I5226671d789d38d67a40628d6886293d00f659fc
        Gerrit-Change-Number: 732000
        Gerrit-PatchSet: 2
        Gerrit-Owner: Peter Weinberger <p...@google.com>
        Gerrit-Reviewer: Alan Donovan <adon...@google.com>
        Gerrit-Reviewer: Peter Weinberger <p...@google.com>
        Gerrit-Attention: Alan Donovan <adon...@google.com>
        Gerrit-Comment-Date: Tue, 23 Dec 2025 15:58:16 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Alan Donovan <adon...@google.com>
        unsatisfied_requirement
        satisfied_requirement
        open
        diffy

        Alan Donovan (Gerrit)

        unread,
        Dec 23, 2025, 12:27:57 PM (yesterday) Dec 23
        to Peter Weinberger, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
        Attention needed from Peter Weinberger

        Alan Donovan voted and added 1 comment

        Votes added by Alan Donovan

        Code-Review+2

        1 comment

        Patchset-level comments
        File-level comment, Patchset 2 (Latest):
        Alan Donovan . resolved

        Thanks!

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Peter Weinberger
        Submit Requirements:
        • requirement satisfiedCode-Review
        • requirement satisfiedNo-Unresolved-Comments
        • requirement satisfiedReview-Enforcement
        • requirement satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: tools
        Gerrit-Branch: master
        Gerrit-Change-Id: I5226671d789d38d67a40628d6886293d00f659fc
        Gerrit-Change-Number: 732000
        Gerrit-PatchSet: 2
        Gerrit-Owner: Peter Weinberger <p...@google.com>
        Gerrit-Reviewer: Alan Donovan <adon...@google.com>
        Gerrit-Reviewer: Peter Weinberger <p...@google.com>
        Gerrit-Attention: Peter Weinberger <p...@google.com>
        Gerrit-Comment-Date: Tue, 23 Dec 2025 17:27:54 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        open
        diffy
        Reply all
        Reply to author
        Forward
        0 new messages