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

[Go] Add more frames to Checked Allocator memory leak output #35212

Closed
hermanschaaf opened this issue Apr 18, 2023 · 2 comments · Fixed by #35215
Closed

[Go] Add more frames to Checked Allocator memory leak output #35212

hermanschaaf opened this issue Apr 18, 2023 · 2 comments · Fixed by #35215

Comments

@hermanschaaf
Copy link
Contributor

hermanschaaf commented Apr 18, 2023

Describe the enhancement requested

The CheckedAllocator output right now provides a single line of context, roughly indicating which type was involved. This is fine for testing within Arrow itself, but in large applications this is often not enough to narrow down where a memory leak is coming from. It would be very helpful if the full call stack could be provided in the output.

It is technically possible to repeatedly run with different values of ARROW_CHECKED_ALLOC_FRAMES and ARROW_CHECKED_REALLOC_FRAMES but this is quite tedious.

Component(s)

Go

@hermanschaaf
Copy link
Contributor Author

I have an implementation of this (and it's been very useful) but not sure how we'd want the details to look. I might open a draft PR for feedback soon

@zeroshade
Copy link
Member

Sounds good @hermanschaaf I'd be happy to look at it when you submit the draft PR. If it doesn't automatically add me to it just tag me on it.

zeroshade pushed a commit that referenced this issue Apr 18, 2023
…_MAX_RETAINED_FRAMES (#35215)

This adds support for printing the full call stack when a leak is reported by the checked memory allocator. An `ARROW_CHECKED_MAX_RETAINED_FRAMES` environment variable controls how many frames are retained. To keep this completely backwards-compatible, the default right now is zero. In this case, the reported error is exactly the same as before. When a higher value is given though, a longer call stack is printed. For example:

Before (same as `ARROW_CHECKED_MAX_RETAINED_FRAMES=0`):

```
checked_allocator.go:160: LEAK of 64 bytes FROM github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).newData line 2396
```

After (with `ARROW_CHECKED_MAX_RETAINED_FRAMES=100`):

```
checked_allocator.go:160: LEAK of 64 bytes FROM github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).newData line 2396
     github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).NewTimestampArray line 2386
     github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).NewArray line 2380
     github.com/apache/arrow/go/v12/arrow/array.(*RecordBuilder).NewRecord line 346
     github.com/cloudquery/plugin-sdk/v2/testdata.GenTestData line 243
     github.com/cloudquery/plugin-sdk/v2/plugins/destination.testMigration line 53
     github.com/cloudquery/plugin-sdk/v2/plugins/destination.(*PluginTestSuite).destinationPluginTestMigrate.func5 line 239
     testing.tRunner line 1576
     runtime.goexit line 1172
```
* Closes: #35212

Authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
@zeroshade zeroshade added this to the 13.0.0 milestone Apr 18, 2023
liujiacheng777 pushed a commit to LoongArch-Python/arrow that referenced this issue May 11, 2023
…HECKED_MAX_RETAINED_FRAMES (apache#35215)

This adds support for printing the full call stack when a leak is reported by the checked memory allocator. An `ARROW_CHECKED_MAX_RETAINED_FRAMES` environment variable controls how many frames are retained. To keep this completely backwards-compatible, the default right now is zero. In this case, the reported error is exactly the same as before. When a higher value is given though, a longer call stack is printed. For example:

Before (same as `ARROW_CHECKED_MAX_RETAINED_FRAMES=0`):

```
checked_allocator.go:160: LEAK of 64 bytes FROM github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).newData line 2396
```

After (with `ARROW_CHECKED_MAX_RETAINED_FRAMES=100`):

```
checked_allocator.go:160: LEAK of 64 bytes FROM github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).newData line 2396
     github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).NewTimestampArray line 2386
     github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).NewArray line 2380
     github.com/apache/arrow/go/v12/arrow/array.(*RecordBuilder).NewRecord line 346
     github.com/cloudquery/plugin-sdk/v2/testdata.GenTestData line 243
     github.com/cloudquery/plugin-sdk/v2/plugins/destination.testMigration line 53
     github.com/cloudquery/plugin-sdk/v2/plugins/destination.(*PluginTestSuite).destinationPluginTestMigrate.func5 line 239
     testing.tRunner line 1576
     runtime.goexit line 1172
```
* Closes: apache#35212

Authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
ArgusLi pushed a commit to Bit-Quill/arrow that referenced this issue May 15, 2023
…HECKED_MAX_RETAINED_FRAMES (apache#35215)

This adds support for printing the full call stack when a leak is reported by the checked memory allocator. An `ARROW_CHECKED_MAX_RETAINED_FRAMES` environment variable controls how many frames are retained. To keep this completely backwards-compatible, the default right now is zero. In this case, the reported error is exactly the same as before. When a higher value is given though, a longer call stack is printed. For example:

Before (same as `ARROW_CHECKED_MAX_RETAINED_FRAMES=0`):

```
checked_allocator.go:160: LEAK of 64 bytes FROM github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).newData line 2396
```

After (with `ARROW_CHECKED_MAX_RETAINED_FRAMES=100`):

```
checked_allocator.go:160: LEAK of 64 bytes FROM github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).newData line 2396
     github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).NewTimestampArray line 2386
     github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).NewArray line 2380
     github.com/apache/arrow/go/v12/arrow/array.(*RecordBuilder).NewRecord line 346
     github.com/cloudquery/plugin-sdk/v2/testdata.GenTestData line 243
     github.com/cloudquery/plugin-sdk/v2/plugins/destination.testMigration line 53
     github.com/cloudquery/plugin-sdk/v2/plugins/destination.(*PluginTestSuite).destinationPluginTestMigrate.func5 line 239
     testing.tRunner line 1576
     runtime.goexit line 1172
```
* Closes: apache#35212

Authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
rtpsw pushed a commit to rtpsw/arrow that referenced this issue May 16, 2023
…HECKED_MAX_RETAINED_FRAMES (apache#35215)

This adds support for printing the full call stack when a leak is reported by the checked memory allocator. An `ARROW_CHECKED_MAX_RETAINED_FRAMES` environment variable controls how many frames are retained. To keep this completely backwards-compatible, the default right now is zero. In this case, the reported error is exactly the same as before. When a higher value is given though, a longer call stack is printed. For example:

Before (same as `ARROW_CHECKED_MAX_RETAINED_FRAMES=0`):

```
checked_allocator.go:160: LEAK of 64 bytes FROM github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).newData line 2396
```

After (with `ARROW_CHECKED_MAX_RETAINED_FRAMES=100`):

```
checked_allocator.go:160: LEAK of 64 bytes FROM github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).newData line 2396
     github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).NewTimestampArray line 2386
     github.com/apache/arrow/go/v12/arrow/array.(*TimestampBuilder).NewArray line 2380
     github.com/apache/arrow/go/v12/arrow/array.(*RecordBuilder).NewRecord line 346
     github.com/cloudquery/plugin-sdk/v2/testdata.GenTestData line 243
     github.com/cloudquery/plugin-sdk/v2/plugins/destination.testMigration line 53
     github.com/cloudquery/plugin-sdk/v2/plugins/destination.(*PluginTestSuite).destinationPluginTestMigrate.func5 line 239
     testing.tRunner line 1576
     runtime.goexit line 1172
```
* Closes: apache#35212

Authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants