fix memory leak #45
fix memory leak #45
Conversation
psychocoderHPC
force-pushed
the
fix-memoryLeak
branch
4 times, most recently
from
dce7820
to
65363a3
Compare
redGrapes/util/chunked_list.hpp
Outdated
| @@ -409,9 +406,14 @@ struct ChunkedList | |||
| ChunkedList( size_t est_chunk_size = 32 ) | |||
| : ChunkedList( | |||
| Allocator(), | |||
| est_chunk_size | |||
There was a problem hiding this comment.
@michaelsippel For me it is not clear why this constructor's implementation is different to the second constructor. Why do you not compute the chunk_size in the previous version before my PR.
This means for this constructor the member was initialized to zero.
There was a problem hiding this comment.
The implementation is not different, the first one just uses the default allocator (picks the allocator assigned to the current worker) and is implemented by calling the second one
There was a problem hiding this comment.
ohh yes, thanks for the clarification. I got confused by the words chunk, chunks, Chunk, ...^^ everywhere^^ (my fault)
psychocoderHPC
force-pushed
the
fix-memoryLeak
branch
from
65363a3
to
b3e835a
Compare
readGrapes init and finalize is missing in `1_resource.cpp`
psychocoderHPC
force-pushed
the
fix-memoryLeak
branch
from
b3e835a
to
b724803
Compare
With ComputationalRadiationPhysics#44 we tried to fix the memory leak which can happen if more than one thread is allocating a new chunk. The PR ComputationalRadiationPhysics#44 was not fixing the issue and introduced a double free issue.
psychocoderHPC
force-pushed
the
fix-memoryLeak
branch
from
b724803
to
bc797f7
Compare
|
@michaelsippel I applied your review comments and rebased against #47 |
I am fine if we refactor these parts in a follow up but as long as there is no PR fix that instantly fixes the current issue of memory leaks I would like to bring it in to have a version that is working correctly. I am also fine if this PR is not going in, both ways are possible as long as the dev branch becomes usable again. |
|
For testing we should use small chunk sizes with many tasks since otherwise when everything fits into one chunk, the implementation will not be stressed |
|
Unfortunately, this PR creates a deadlock :-( during testing |
Equally to ComputationalRadiationPhysics#45 this PR should solve the possible double free and double alloc.
Equally to ComputationalRadiationPhysics#45 this PR should solve the possible double free and double alloc.
|
now already included in #48 |
With #44 we tried to fix the memory leak which can happen if more than one thread is allocating a new chunk.
The PR #44 was not fixing the issue and introduced a double free issue.
@michaelsippel I have not tested the PR and wrote it blindly, we need to perform a runtime test before merging.