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

#0: update allocator search option and leak check #4457

Closed
wants to merge 4 commits into from
Closed

Conversation

muthutt
Copy link
Contributor

@muthutt muthutt commented Dec 20, 2023

#0: update allocator search option and leak check
: fix the allocator best search strategy which had a bug - it was same as first search strategy
: fix memory leak (assign on std::make_unique() should ideally use std::move() or reset(new T(...)) etc.)

Valgrind report on test, (python_env) ~/tt-metal-concat$ valgrind --leak-check=full ./build/test/tt_metal/test_reduce_h shows:

==61028== Warning: set address range perms: large range [0x6654000, 0x23654000) (noaccess)
==61028== 
==61028== HEAP SUMMARY:
==61028==     in use at exit: 839,285 bytes in 11,330 blocks
==61028==   total heap usage: 579,056 allocs, 567,726 frees, 1,220,583,992 bytes allocated
==61028== 
==61028== 192 (96 direct, 96 indirect) bytes in 2 blocks are definitely lost in loss record 182 of 373
==61028==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==61028==    by 0x4A8CA49: tt::tt_metal::allocator::FreeList::init() (free_list.cpp:23)
==61028==    by 0x4A8E2B0: tt::tt_metal::allocator::FreeList::FreeList(unsigned long, unsigned long, unsigned long, unsigned long, tt::tt_metal::allocator::FreeList::SearchPolicy) (free_list.cpp:19)
==61028==    by 0x4A8F482: make_unique<tt::tt_metal::allocator::FreeList, long unsigned int&, long unsigned int&, unsigned int const&, unsigned int const&, tt::tt_metal::allocator::FreeList::SearchPolicy> (unique_ptr.h:857)
==61028==    by 0x4A8F482: tt::tt_metal::allocator::BankManager::init_allocator(unsigned long, unsigned long) (allocator.cpp:20)
==61028==    by 0x4A90F1B: tt::tt_metal::allocator::BankManager::BankManager(tt::tt_metal::BufferType const&, std::vector<long, std::allocator<long> > const&, unsigned long, unsigned long) (allocator.cpp:49)
==61028==    by 0x4A91135: tt::tt_metal::allocator::init_one_bank_per_channel(tt::tt_metal::Allocator&, tt::tt_metal::AllocatorConfig const&) (allocator.cpp:159)
==61028==    by 0x4A9048D: operator() (std_function.h:688)

The fixed version shows leak as,

==63433== LEAK SUMMARY:
==63433==    definitely lost: 840 bytes in 4 blocks
==63433==    indirectly lost: 149,818 bytes in 1,986 blocks
==63433==      possibly lost: 0 bytes in 0 blocks
==63433==    still reachable: 688,627 bytes in 9,340 blocks
==63433==         suppressed: 0 bytes in 0 blocks
==63433== Reachable blocks (those to which a pointer was found) are not shown.
==63433== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==63433== 
==63433== For lists of detected and suppressed errors, rerun with: -s

@muthutt
Copy link
Contributor Author

muthutt commented Dec 21, 2023

@muthutt muthutt force-pushed the ma/newloc branch 2 times, most recently from 418fc60 to 7289081 Compare January 30, 2024 22:57
Muthu added 4 commits February 1, 2024 22:21
   : fix the allocator best search strategy which had a bug - it was same as first search strategy
@muthutt
Copy link
Contributor Author

muthutt commented Feb 6, 2024

@abhullar-tt - lets chat sometime this week with intention to handoff this branch (or find a way to move conversation) and until then I'm going to close this PR.

@muthutt muthutt closed this Feb 6, 2024
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