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

runtime: document that MemStats.Lookups is always zero #55915

Closed
zc24 opened this issue Sep 28, 2022 · 13 comments
Closed

runtime: document that MemStats.Lookups is always zero #55915

zc24 opened this issue Sep 28, 2022 · 13 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Documentation NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@zc24
Copy link

zc24 commented Sep 28, 2022

package main
import "fmt"
import "runtime"
func main () {
var a = runtime.MemStats{}
runtime.ReadMemStats(&a)
fmt.Printf("%+v\n", a.Lookups)
}

@dmitshur
Copy link
Contributor

dmitshur commented Sep 28, 2022

Please include answers to questions from the issue template:

  • What did you do?
  • What did you expect to see?
  • What operating system and processor architecture are you using (go env)?
  • Does this issue reproduce with the latest release?

Note that the release policy means there will be minor releases for Go 1.19 and 1.18, not older.

@dmitshur dmitshur added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Sep 28, 2022
@zc24
Copy link
Author

zc24 commented Sep 29, 2022

@dmitshur hello,I want to get the value of runtime.MEMstats.Lookups in the latest version 1.19, but It's always going to be 0. I see that in the 1.11 submission history, someone changed this, and in the end, lookups was discarded, but the reason is not explained clearly.I want to know the reason for the abandonment

@go101
Copy link

go101 commented Sep 29, 2022

Yes, 1.10 prints 3 (on linux/amd64). But many runtime internal implementation details change from version to version. So this is not a sufficient evidence to prove the field is discarded.

@dmitshur dmitshur changed the title why runtime.MemStats.lookup Is always zero at 1.11.13 ,but 1.10 not like that runtime: MemStats.Lookups is always zero but not documented as such; it used to be populated prior to Go 1.11 Sep 29, 2022
@gopherbot gopherbot added compiler/runtime Issues related to the Go compiler and/or runtime. Documentation labels Sep 29, 2022
@dmitshur dmitshur removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Sep 29, 2022
@zc24
Copy link
Author

zc24 commented Sep 29, 2022

@go101 @dmitshur Which method will trigger the increase of Lookups in the latest version 1.19.I don't seem to be able to find anywhere that would modify the value of lookups effectively

@go101
Copy link

go101 commented Sep 29, 2022

It does looks the Lookups field is never used as L-values now.

@ZekeLu
Copy link
Contributor

ZekeLu commented Sep 29, 2022

I think this is changed by 41e6abd. The comment for the next field was changed in that commit:

nlookup uint64 // number of pointer lookups (unused)

-   nlookup     uint64 // number of pointer lookups
+   nlookup     uint64 // number of pointer lookups (unused)

@zc24
Copy link
Author

zc24 commented Sep 29, 2022

@ZekeLu I looked at the submission information, but I couldn't see the fundamental reason why the lookups count was dropped, perhaps because the statistic itself doesn't mean much.The field is still there for downward compatibility

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 29, 2022
@dmitshur dmitshur added this to the Backlog milestone Sep 29, 2022
@dmitshur
Copy link
Contributor

dmitshur commented Sep 29, 2022

@mknyszek Is this something you're familiar with? The issue here seems to be that the MemStats.Lookups is never populated on tip, and its internal comment on mstats.nlookup says it's unused. However, the publicly visible comment doesn't indicate that it's deprecated or unused, just that it's "useful for debugging runtime internals".

Does the documentation need to be updated, or is its current description fine and this is working as intended?

@prattmic
Copy link
Member

prattmic commented Oct 5, 2022

I believe this just needs a documentation update to say the field is deprecated.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 6, 2022
@mknyszek
Copy link
Contributor

What Michael said is correct, the documentation is just out of date. As for why it was set to zero, one reason why might be that this needs to be tracked on a very hot path in the runtime, so it's a cost everyone would pay for.

I'll update the docs.

@mknyszek mknyszek modified the milestones: Backlog, Go1.20 Oct 21, 2022
@dmitshur dmitshur changed the title runtime: MemStats.Lookups is always zero but not documented as such; it used to be populated prior to Go 1.11 runtime: document that MemStats.Lookups is always zero Oct 21, 2022
@gopherbot gopherbot modified the milestones: Go1.20, Go1.21 Feb 1, 2023
@gopherbot
Copy link

Change https://go.dev/cl/502155 mentions this issue: runtime: mark MemStats.Lookups as deprecated

@mknyszek mknyszek changed the title runtime: document that MemStats.Lookups is always zero proposal: runtime: retroactively deprecate MemStats.Lookups for Go 1.11 Jun 10, 2023
@mknyszek mknyszek changed the title proposal: runtime: retroactively deprecate MemStats.Lookups for Go 1.11 runtime: document that MemStats.Lookups is always zero Jun 10, 2023
@mknyszek
Copy link
Contributor

This field has basically been deprecated since Go 1.11. I don't know the procedure we should follow for deprecating it. I filed #60714 to find out.

@mknyszek
Copy link
Contributor

I think this is basically blocked on #60714. That will decide the ultimate fate of MemStats.Lookups. It doesn't look like this is going to happen for Go 1.21 either (though, it's definitely not urgent).

Closing in favor of #60714.

@mknyszek mknyszek closed this as not planned Won't fix, can't repro, duplicate, stale Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Documentation NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants