x/tools/gopls: fieldalignment analyzer reports confusing "errors" about memory layout #67762
Comments
new001code
added
gopls
|
I didn't get the same hint with your file |
|
I don't know what ide you're using. I use neovim + lazyvim, and I use gopls. I tested golang or vscode and didn't prompt for memory alignment.
…---- Replied Message ----
| From | ***@***.***> |
| Date | 06/02/2024 01:10 |
| To | ***@***.***> |
| Cc | ***@***.***>***@***.***> |
| Subject | Re: [golang/go] x/tools/gopls: issue Gopls gives errors about the memory occupancy in the structure (Issue #67762) |
I didn't get the same hint with your file
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
The message you're seeing comes from gopls' fieldalignment analyzer. It is not enabled by default, which is why others are not seeing it; you must have explicitly enabled it in your gopls configuration. The reason it is disabled by default is that its false positive rate is almost 100%: nearly every time it reports a diagnostic, it is not due to a mistake in your code. A suboptimal field ordering may be desirable for various reasons, first among them clarity. Only in the tiny handful of data structures that show up at the top of a profile is there any need to consider field ordering, and that's not within the power of an analyzer to deduce. Also, the diagnostics produced by the analyzer are very confusing: there are two kinds, one which refers to the size of the struct, and the other which uses the term "pointer bytes". This term is intended to refer to the prefix of the object that contains pointers and must therefore be scanned by the garbage collector. Even though I was familiar with the concept, the phrase "pointer bytes" did not make me think of it. I suspect very few users understand the working of the GC in sufficient detail to understand this term, and not be confused by it as both you and I were in different ways. The upcoming gopls v0.16 will have a new feature: hovering over the declaring identifier of a struct type, or one of its field declarations, will reveal the type size, alignment, and offset as part of the hover information. This seems to me to be a more useful way to surface the information as it is discreet and unobtrusive and can be sought on demand by the user acting on the information provided by a heap profile. I propose that we delete the fieldalignment analyzer altogether. |
adonovan
changed the title
|
yep i see.Thank you for sharing.I also look forward to the change made by gopls v0.16
…---- Replied Message ----
| From | Alan ***@***.***> |
| Date | 06/03/2024 20:11 |
| To | ***@***.***> |
| Cc | ***@***.***>***@***.***> |
| Subject | Re: [golang/go] x/tools/gopls: issue Gopls gives errors about the memory occupancy in the structure (Issue #67762) |
The message you're seeing comes from gopls' fieldalignment analyzer. It is not enabled by default, which is why others are not seeing it; you must have explicitly enabled it in your gopls configuration.
The reason it is disabled by default is that its false positive rate is almost 100%: nearly every time it reports a diagnostic, it is not due to a mistake in your code. A suboptimal field ordering may be desirable for various reasons, first among them clarity. Only in the tiny handful of data structures that show up at the top of a profile is there any need to consider field ordering, and that's not within the power of an analyzer to deduce.
Also, the diagnostics produced by the analyzer are very confusing: there are two kinds, one which refers to the size of the struct, and the other which uses the term "pointer bytes". This term is intended to refer to the prefix of the object that contains pointers and must therefore be scanned by the garbage collector. Even though I was familiar with the concept, the phrase "pointer bytes" did not make me think of it. I suspect very few users understand the working of the GC in sufficient detail to understand this term, and not be confused by it as both you and I were in different ways.
The upcoming gopls v0.16 will have a new feature: hovering over the declaring identifier of a struct type, or one of its field declarations, will reveal the type size, alignment, and offset as part of the hover information. This seems to me to be a more useful way to surface the information as it is discreet and unobtrusive and can be sought on demand by the user acting on the information provided by a heap profile.
I propose that we delete the fieldalignment analyzer altogether.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
Another option is to make the severity of this analyzer "hint", which decreases the visibility in many LSP clients. The fieldalignment analyzer was added in https://go.dev/cl/270217, as a public package in x/tools, without a proposal or even associated issue. We were more lax about the proposal process in those days, but as an unfortunate result we cannot simply delete this analyzer. We'd have to carve out a module, tag, and delete. |
|
Change https://go.dev/cl/590375 mentions this issue: |
I enjoyed this analyzer and had it enabled by default. It was OK for me to ignore the warnings when they didn't apply (though hints would have been better, as mentioned by @findleyr). I've encountered a number of situations where I could significantly reduce the size of structs without impacting readability. I realize I'm an outlier here.
I'm wondering what the problem is/was though. This analyzer was optional. One has to go out of their way to enable it. Is there a large maintenance burden carrying it around? Would it be better to have this analyzer as a lens perhaps? IIRC, lenses are meant to be toggled at edit-time, which would make this analyzer even less obtrusive. In this context, I like the GC escape information lens. I like that one too, I hope it doesn't get taken away. By the way, the deprecation message led me to the wrong issue:
|
gopls version
golang.org/x/tools/gopls v0.15.3
go env
What did you do?
I defined a structure.
type ApiResponse struct {
Success bool
json:"success"Code string
json:"code"Message string
json:"message"Data interface{}
json:"data"}
What did you see happen?
Gave me a hint: "struct with 56 pointer bytes could be 40 [default]"
What did you expect to see?
I adjusted to the field order of the structure expected by gopls, but the memory size of the structure should still be 56 instead of 40.
package main
import (
"fmt"
"testing"
"unsafe"
)
type ApiResponse struct {
Data interface{}
json:"data"Code string
json:"code"Message string
json:"message"Success bool
json:"success"}
func TestStruct(t *testing.T) {
fmt.Println(unsafe.Sizeof(ApiResponse{}))
fmt.Println(unsafe.Alignof(ApiResponse{}))
}
Editor and settings
No response
Logs
No response
The text was updated successfully, but these errors were encountered: