[GitHub] [arrow] zeroshade commented on a diff in pull request #35973: GH-35015: [Go] Fix parquet memleak

["zeroshade (via GitHub)" <gi...@apache.org>]

zeroshade commented on code in PR #35973:
URL: https://github.com/apache/arrow/pull/35973#discussion_r1222041752


##########
go/arrow/memory/checked_allocator.go:
##########
@@ -160,14 +160,24 @@ func (a *CheckedAllocator) AssertSize(t TestingT, sz int) {
 				break
 			}
 			callersMsg.WriteString("\t")
-			callersMsg.WriteString(frame.Function)
-			callersMsg.WriteString(fmt.Sprintf(" line %d", frame.Line))
+			if fn := frame.Func; fn != nil {
+				callersMsg.WriteString(fmt.Sprintf("%s+%x", fn.Name(), frame.PC-fn.Entry()))

Review Comment:
   add this as a comment here so that we don't lose the information? This is good to know so we don't look at this in the future and go "wtf is this doing!"



##########
go/arrow/memory/buffer.go:
##########
@@ -35,7 +35,7 @@ type Buffer struct {
 
 // NewBufferBytes creates a fixed-size buffer from the specified data.
 func NewBufferBytes(data []byte) *Buffer {
-	return &Buffer{refCount: 0, buf: data, length: len(data)}
+	return &Buffer{refCount: 1, buf: data, length: len(data)}

Review Comment:
   So the interesting aspect about this is that the `refCount` on this will never get reduced to 0 because in this situation `Release` and `Retain` are no-ops (because `b.mem` and `b.parent` are both `nil`). So we should probably leave this as 0, not 1.



##########
go/arrow/memory/checked_allocator.go:
##########
@@ -160,14 +160,24 @@ func (a *CheckedAllocator) AssertSize(t TestingT, sz int) {
 				break
 			}
 			callersMsg.WriteString("\t")
-			callersMsg.WriteString(frame.Function)
-			callersMsg.WriteString(fmt.Sprintf(" line %d", frame.Line))
+			if fn := frame.Func; fn != nil {
+				callersMsg.WriteString(fmt.Sprintf("%s+%x", fn.Name(), frame.PC-fn.Entry()))
+			} else {
+				callersMsg.WriteString(fmt.Sprintf("%s, line %d", frame.Function, frame.Line))
+			}
+			callersMsg.WriteString("\n\t\t")
+			callersMsg.WriteString(frame.File + ":" + strconv.Itoa(frame.Line))

Review Comment:
   same as my previous comment, make this a comment in the code so that we know what this is and why it's there :smile:



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org