-
Notifications
You must be signed in to change notification settings - Fork 415
tight floor plan memory leak bugs #2178
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
Conversation
Move from PR #2161 , continue to fix the memory leak bugs. |
memory leak bugs list: Incr Criticality updates 0 in 0 sec ================================================================= Direct leak of 88200 byte(s) in 3675 object(s) allocated from: Direct leak of 336 byte(s) in 1 object(s) allocated from: Direct leak of 160 byte(s) in 1 object(s) allocated from: Indirect leak of 56604448 byte(s) in 1497270 object(s) allocated from: Indirect leak of 39085147 byte(s) in 206479 object(s) allocated from: Indirect leak of 32636176 byte(s) in 150562 object(s) allocated from: Indirect leak of 19700200 byte(s) in 3675 object(s) allocated from: Indirect leak of 6061600 byte(s) in 189425 object(s) allocated from: Indirect leak of 2342136 byte(s) in 189425 object(s) allocated from: Indirect leak of 1204496 byte(s) in 119604 object(s) allocated from: Indirect leak of 470400 byte(s) in 3675 object(s) allocated from: Indirect leak of 307168 byte(s) in 9599 object(s) allocated from: Indirect leak of 76864 byte(s) in 6 object(s) allocated from: Indirect leak of 10112 byte(s) in 316 object(s) allocated from: Indirect leak of 64 byte(s) in 2 object(s) allocated from: Indirect leak of 32 byte(s) in 1 object(s) allocated from: SUMMARY: AddressSanitizer: 158587539 byte(s) leaked in 2373716 allocation(s). |
|
update the memory leaks' log: ================================================================= Direct leak of 336 byte(s) in 1 object(s) allocated from: Direct leak of 160 byte(s) in 1 object(s) allocated from: Indirect leak of 39085147 byte(s) in 206479 object(s) allocated from: Indirect leak of 32636176 byte(s) in 150562 object(s) allocated from: Indirect leak of 1204496 byte(s) in 119604 object(s) allocated from: Indirect leak of 470400 byte(s) in 3675 object(s) allocated from: Indirect leak of 307264 byte(s) in 9602 object(s) allocated from: Indirect leak of 76864 byte(s) in 6 object(s) allocated from: Indirect leak of 10112 byte(s) in 316 object(s) allocated from: SUMMARY: AddressSanitizer: 73790955 byte(s) leaked in 490246 allocation(s). |
|
update the memory leaks' logs: ================================================================= Direct leak of 160 byte(s) in 1 object(s) allocated from: Indirect leak of 39085147 byte(s) in 206479 object(s) allocated from: Indirect leak of 32636176 byte(s) in 150562 object(s) allocated from: Indirect leak of 1204496 byte(s) in 119604 object(s) allocated from: Indirect leak of 470400 byte(s) in 3675 object(s) allocated from: SUMMARY: AddressSanitizer: 73396379 byte(s) leaked in 480321 allocation(s). |
In regression test task "vpr_ispd" of "vtr_reg_nightly_test5", it seems like the memory usage is 0.6062963593520936 time of golden, which is outside of the range [0.8,1.203]. The reason should be that after fixing the memory leak of cluster_placement_stats, memory usage is greatly reduced. Could I update the golden results? @vaughnbetz |
Yes, that's a great improvement so fine to update the golden result. |
|
update memory leaks' log: ================================================================= Indirect leak of 39085147 byte(s) in 206479 object(s) allocated from: Indirect leak of 32636176 byte(s) in 150562 object(s) allocated from: Indirect leak of 1204496 byte(s) in 119604 object(s) allocated from: Indirect leak of 470400 byte(s) in 3675 object(s) allocated from: SUMMARY: AddressSanitizer: 73396219 byte(s) leaked in 480320 allocation(s). |
|
All memory leaks are clear now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me; just suggest deleting the commented out code.
…constraint xml file
…constraint xml file, ignore last commit, change wrong place
Failed reason: when reading vpr constraint xml file, and loading int, encounter errno=22, which means invalid argument. |
Problem reason: Try to solve directly: Final solution: |
Now all the checks passed. |
Create a new test "strong_tight_floorplan" in vtr_reg_strong for sanitizer test. The new test case using benchmark "bigkey.blif" and architecture "k6_N10_40nm.xml" with a vpr placement constraint file "bigkey_tight_floorplan.xml". This testcase can test the tight floor plan relevant codes to make sure no memory leaks on this part in the furture. |
Description
Related Issue
This tasks continues from PR #2161.
The runtime of this new testcase with make sanitizer on our check server is 336.56s, without sanitizer is 11s.
The global variable "errno" from the GNU C Library "errno.h" is set to 22 because: when invoking"out.set_add_atom_name_pattern(attr.value(), context);" at "vpr_constraints_uxsdcxxx.h:480" in function "inline void load_add_atom(const pugi::xml_node& root, T& out, Context& context, const std::function<void(const char*)>* report_error, ptrdiff_t* offset_debug)", the argument "context" is not used. So the global variable "errno" is set to 22 which means "invalid argument" when making with sanitizer on our check server. Thus, when coming into function load_int, the errno is 22 although there is no problem when parsing int value from a string. This might because of the difference on g++ version or other environment values between the check server with our wintermute server.
Try to solve directly:
This problem cannot be solved by fixing the problem on the argument of the function set_add_atom_name_pattern. Because the variable "context" has been declared as "void*" which is correct, and the function "set_add_atom_name_pattern" belong to "class VprConstraintsSerializer" in file "vpr_constraints_serializer.h", and this class inherits from a template class "class VprConstraintsBase". In the base class, the function "set_add_atom_name_pattern" has already been defined as a pure virtual function with template "virtual inline void set_add_atom_name_pattern(const char* name_pattern, typename ContextTypes::AddAtomWriteContext& ctx) = 0;" Although the argument "context" is not used in the derived class "VprConstraintsSerializer", we cannot change the function definition and we cannot give it a default value "NULL" because "context" is a reference of void pointer.
Final solution:
A better and more reasonable solution is that we set "errno = 0" before using the global variable "errno". That is, we give a statement "errno = 0;" before transfering a string value to an int value. This is also the correct way to use global variable "errno".
Motivation and Context
How Has This Been Tested?
Use task vpr_tight_floorplan in regression_test vtr_reg_nightly_test5.
Use task strong_tight_floorplan in regression_test vtr_reg_strong for faster sanitizer test.
Types of changes
Checklist: