Closed
Bug 398397
Opened 17 years ago
Closed 17 years ago
Memory leak checking script doesn't count bytes/blocks correctly.
Categories
(NSS :: Test, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
3.11.9
People
(Reporter: slavomir.katuscak+mozilla, Assigned: slavomir.katuscak+mozilla)
Details
Attachments
(2 files)
1.75 KB,
patch
|
christophe.ravel.bugs
:
review+
nelson
:
review+
|
Details | Diff | Splinter Review |
11.58 KB,
patch
|
nelson
:
review+
julien.pierre
:
review+
|
Details | Diff | Splinter Review |
There are more different messages printed when memory is leaked or in use, script is now checking only for one of them, need to check for all.
Assignee | ||
Comment 1•17 years ago
|
||
Attachment #283348 -
Flags: review?(christophe.ravel.bugs)
Assignee | ||
Updated•17 years ago
|
Attachment #283348 -
Flags: review?(nelson)
Assignee | ||
Comment 2•17 years ago
|
||
Please review this patch ASAP (2 reviews), we need to get correct numbers from all platforms from both trunk and branch.
Comment 3•17 years ago
|
||
Comment on attachment 283348 [details] [diff] [review] Patch v1. The parsing of the output could be optimized (by using awk for example). In the meantime, this patch seems to be good enough with the current line by line parsing.
Attachment #283348 -
Flags: review?(christophe.ravel.bugs) → review+
Comment 4•17 years ago
|
||
Comment on attachment 283348 [details] [diff] [review] Patch v1. Slavo, add a comment here when this patch has been committed on the trunk. The comment should record the date and time of the commit, and also the version number(s) of the file(s) committed. When I've seen the effect of this on the trunk, I will review it for the branch.
Comment 5•17 years ago
|
||
see bug 398419 for the awk version of the parsing on Linux. It should be fairly similar on Solaris.
Comment 6•17 years ago
|
||
We need more than an overall total. We need per-process run statistics. I suggest that Tinderbox should publish minimum, maximum, and average numbers of bytes and blocks per process under test.
Priority: -- → P1
Version: 3.12 → trunk
Comment 7•17 years ago
|
||
Comment on attachment 283348 [details] [diff] [review] Patch v1. Need to make the awk version of this script also handle these same strings.
Attachment #283348 -
Flags: review?(nelson) → review+
Comment 8•17 years ago
|
||
To clarify my comment 6, I don't mean that every time we run a process under DBX or valgrind, we need to output those 3 statistics. I mean that when it's all done, those three statistics should be output for the whole run, namely: - the minimum number of bytes and blocks leaked by any process under test, - the maximum number of bytes and blocks leaked by any process under test, - the average number of bytes and blocks leaked by any process under test,
Assignee | ||
Comment 9•17 years ago
|
||
Added statistics that Nelson wanted in previous patch. We are currently testing 3 different tools (selfserv, strsclnt, ocspclnt), so statistics are measured for every tool separately (there is no much sense to compare data of different processes). Because there are too many numbers produced (2 builds x 3 processes x 2 parameters x 3 values), I suggest to print statistics only to output.log, not directly to Tinderbox results page. In this patch I also fixed some awk indenting and style, to not mess more styles and use only one.
Attachment #285107 -
Flags: review?(nelson)
Comment 10•17 years ago
|
||
Comment on attachment 285107 [details] [diff] [review] Statistics of leaks (min, max, avg). Slavo, please attach to this bug a file of sample output from this patch. Thanks.
Assignee | ||
Comment 11•17 years ago
|
||
Nelson, try to check this Tinderboex logfile (not integrated yet, only this one run was patched) and search there for string 'statistics': http://tinderbox.mozilla.org/showlog.cgi?log=NSS/1192791720.1192817006.21707.gz Areas related to this bug: Selfserv statistics: Leaked bytes: 2996 min, 3117 avg, 3178 max Leaked blocks: 34 min, 34 avg, 34 max Total runs: 9 Strsclnt statistics: Leaked bytes: 3450 min, 3518 avg, 3552 max Leaked blocks: 46 min, 48 avg, 52 max Total runs: 240 Ocspclnt statistics: Leaked bytes: 3461 min, 3506 avg, 3552 max Leaked blocks: 60 min, 62 avg, 64 max Total runs: 4 The log is relatively long and you will find those areas 4 times there, because we run there DBG and OPT build and now also versions without and with PKIX.
Comment 12•17 years ago
|
||
Comment on attachment 285107 [details] [diff] [review] Statistics of leaks (min, max, avg). r=nelson, on the basis of the sample output shown in comment 11.
Attachment #285107 -
Flags: review?(nelson) → review+
Assignee | ||
Comment 13•17 years ago
|
||
Checking in memleak.sh; /cvsroot/mozilla/security/nss/tests/memleak/memleak.sh,v <-- memleak.sh new revision: 1.18; previous revision: 1.17 done
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•17 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: --- → 3.11.9
Assignee | ||
Updated•17 years ago
|
Attachment #285107 -
Flags: review?(julien.pierre.boogz)
Assignee | ||
Updated•17 years ago
|
Priority: P1 → P2
Comment 14•17 years ago
|
||
I concur that this is P2 for the 3.11 branch, but it is P1 for the trunk.
Version: trunk → 3.11.7
Assignee | ||
Comment 15•17 years ago
|
||
Nelson, it's already fixed in trunk. Now I want to merge changes also to branch, because after many changes in memleak.sh in trunk it's now more difficult to merge new trunk patches to branch.
Updated•17 years ago
|
Attachment #285107 -
Flags: review?(julien.pierre.boogz) → review+
Assignee | ||
Comment 16•17 years ago
|
||
Added to branch.
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•