Re: [Bug 1359975] pmchart run-away mem leak replaying multi-archive when rewinding

Hi Dave,

----- Original Message -----
> https://bugzilla.redhat.com/show_bug.cgi?id=1359975
> 
> --- Comment #17 from Dave Brolley <[email protected]> ---
> Additional proposed commit (on top of the previous one):
> 
> In the tree/branch:  git://git.pcp.io/brolley/pcp rhbz1359975
> 
> commit 82e02ffea204d26e7617a4bcf4be0b9326c64457
> Author: Dave Brolley <[email protected]>
> Date:   Mon Dec 12 14:39:09 2016 -0500
> 
>     RHBZ 1359975: pmchart run-away mem leak replaying multi-archive when
> rewinding
> 
>     Refinement. Exposed by qa regressions. Ensure that instance domains are
>     in the correct order in the hash chain:
> 
>     - Primary sort by timestamp (descending)
>     - Secondary sort: latest added at the head of each time slot. This
>       includes moving duplicates to the head of their time slot when
>       detected.
> 
>     Before multi-archive contexts and duplicate-filtering, this happened
>     automatically.

Couple things - the new sameindom() routine seems to assuming the instance
IDs and names are sorted in some way (i.e. direct array comparison can be
done, rather than iterating through the 2nd array for each element in the
1st - is that the case?)  Not sure.

Other, small thing - it would be more efficient to add __pmtimevalCmp into
libpcp now I think (its in libpcp_gui for hysterical raisins IIRC), so it's
available for use here instead of using floating point math & comparisons
as is being done for the timestamps now.

Attached patch has some minor pcp-style and typo cleanups.

cheers.

--
Nathan

-=-=-=-=-=-=-=-=-=-=-=-
pcp mailing list
[email protected]
https://groups.io/g/pcp/messages
-=-=-
Groups.io Links:

You receive all messages sent to this group.

View/Reply Online (#14892): https://groups.io/g/pcp/message/14892
View All Messages In Topic (1): https://groups.io/g/pcp/topic/3802235
Mute This Topic: https://groups.io/mt/3802235?uid=174580
New Topic: https://groups.io/g/pcp/post

Change Your Subscription: https://groups.io/g/pcp/editsub?uid=174580

Group Home: https://groups.io/g/pcp
Contact Group Owner: [email protected]

Terms of Service: https://groups.io/static/tos

Unsubscribe: https://groups.io/g/pcp/leave/354243/563757577/xyzzy
-=-=-=-=-=-=-=-=-=-=-=-

diff --git a/src/libpcp/src/logmeta.c b/src/libpcp/src/logmeta.c
index fb6e292..388e501 100644
--- a/src/libpcp/src/logmeta.c
+++ b/src/libpcp/src/logmeta.c
@@ -34,12 +34,15 @@ StrTimeval(const __pmTimeval *tp)
 }
 #endif
 
-/* Return 1 if the indoms are the same, 0 otherwise */
+/*
+ * Return 1 if the indoms are the same, 0 otherwise.
+ * The time stamp does not count for this comparison.
+ */
 static int
-sameindom(const __pmLogInDom *idp1, const __pmLogInDom *idp2) {
+sameindom(const __pmLogInDom *idp1, const __pmLogInDom *idp2)
+{
     int i;
 
-    /* The time stamp does not count for this test. */
     if (idp1->numinst != idp2->numinst)
 	return 0; /* different */
 
@@ -53,9 +56,13 @@ sameindom(const __pmLogInDom *idp1, const __pmLogInDom *idp2) {
     return 1; /* duplicate */
 }
 
-/* Free the given indom. See the comment for the allocation of__pmLogIndom in impl.h */
+/*
+ * Free the given indom.
+ * See the comment for the allocation of__pmLogIndom in impl.h
+ */
 static void
-freeindom(__pmLogInDom *idp) {
+freeindom(__pmLogInDom *idp)
+{
     if (idp->buf) {
 	free(idp->buf);
 	if (idp->allinbuf == 0)
@@ -120,7 +127,7 @@ PM_FAULT_POINT("libpcp/" __FILE__ ":1", PM_FAULT_ALLOC);
      *
      * The indoms need to be sorted by decreasing time stamp. Before
      * multi-archive contexts, this happened automatically. Now we
-     * must do it explicitely. Duplicates must be moved to the head of their
+     * must do it explicitly. Duplicates must be moved to the head of their
      * time slot.
      */
     idp_prev = NULL;
@@ -166,7 +173,7 @@ PM_FAULT_POINT("libpcp/" __FILE__ ":1", PM_FAULT_ALLOC);
 		    /* The duplicate is already in the right place. */
 		    return 0; /* ok */
 		}
-		    
+
 		/* Unlink the duplicate and set it up to be re-inserted. */
 		assert(idp_cached != NULL);
 		if (idp_prev)