Skip to content

Add regression test for memory leaks in string tensors #266

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

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

saudet
Copy link
Contributor

@saudet saudet commented Mar 31, 2021

From #251

Copy link
Collaborator

@karllessard karllessard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @saudet , would you mind explaining quickly what the test is measuring (e.g. where comes the 10_000_000 and why running GC if we are closing explicitly the tensors?)

It could be by adding some comments in the test?

@saudet
Copy link
Contributor Author

saudet commented Apr 1, 2021

It's just some arbitrary value that's probably enough to account for the overhead of the JVM and stuff that it does in the background. There isn't much to explain! I just came up with something that seems to work well enough for a unit test. If you feel that we shouldn't put tests that are not 100% in there, well, let's not merge this. This is the best I could come up with without spending the whole day on it.

@saudet
Copy link
Contributor Author

saudet commented Apr 4, 2021

It looks like there's supposed to be a "CppMemoryChecker" that tracks allocations as per this file in the Python API:
https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/framework/memory_checker.py#L27
However, that module is nowhere to be found. I assume this is something that Google hasn't open sourced...
The only thing available seems to be the "PythonMemoryChecker" that works very much like JavaCPP and is only able to track objects that are registered with its GC:
https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/framework/python_memory_checker.py#L44-L50

@karllessard karllessard merged commit 8392ea7 into tensorflow:master Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants