Hello Monks,

Thank you in advance for your help.
We have an Inline C sub called percent2 that returns the percentiles desired, given a ref to an array of doubles.

($min,$max) = utils::stats::percent2(\@data,0,100);

We are using a temp AV* so that we can sort the data without reordering @data.
The following code was causing a memory leak when called 1000s of times from a 12+ hr-long script:
use Inline C => <<'END_OF_PERCENT2_C_CODE';

#define SvSIOK(sv) ((SvFLAGS(sv) & (SVf_IOK|SVf_IVisUV)) == SVf_IOK)
#define SvNSIV(sv) (SvNOK(sv) ? SvNVX(sv) : (SvSIOK(sv) ? SvIVX(sv) : 
+sv_2nv(sv)))

static I32 S_sv_ncmp(pTHX_ SV *a, SV *b) {
  const NV nv1 = SvNSIV(a);
  const NV nv2 = SvNSIV(b);
  return nv1 < nv2 ? -1 : nv1 > nv2 ? 1 : 0;
}

void percent2(SV* sv, ...) {

  I32 i;
  I32 arrayLen;
  I32 remove_count;
  AV* data;
  AV* data_tmp;
  float value;
  int index;
  SV** pvalue;
  double retval;
  AV* ret;

  ret = newAV();
  data_tmp = newAV();

  Inline_Stack_Vars;
  data = SvRV(Inline_Stack_Item(0));

  if( SvTYPE(data) != SVt_PVAV ) {
    return;
  }

  /* determine the length of the array */
  arrayLen = av_len(data) + 1; 
  if (arrayLen > 0) {

    /* use new tmp array to prevent reordering caller array*/
    for (i = 0; i < arrayLen; i++) {
      pvalue = av_fetch(data,i,0);
      av_push(data_tmp,*pvalue);
    }  
    /* sort array in ascending order */
    sortsv(AvARRAY(data_tmp),arrayLen, S_sv_ncmp);

    /* loop through data array and delete undef entries */
    remove_count = 0;
    for (i = 0; i < arrayLen; i++) {
      /* Fetch the scalar located at i from the array.*/
      pvalue = av_fetch(data_tmp,i,0);
      if (!SvOK(*pvalue)) {
        remove_count++;
      }
      else {
        break;
      }
    }
    for (i = 0; i < remove_count; i++) {
      av_shift(data_tmp);
    }
    arrayLen -= remove_count;
  }

  if (arrayLen > 0) {
    /* loop through percent args and find given value in array */
    for (i = 1; i < Inline_Stack_Items; i++) {
      value = SvNV(Inline_Stack_Item(i));
      if (value <= 100 && value >= 0){
        if (value == 100){
          value -= 1e-13;
        }
        index = (int)((arrayLen-1) * value/100);
        
        /* fetch scalar located at calculated index*/
        pvalue = av_fetch(data_tmp,index,0);
        av_push(ret,*pvalue);
      }
    }
  }   
  /* push into return stack */
  Inline_Stack_Reset;
  arrayLen = av_len(ret) + 1;
  if (arrayLen > 0) {
    for (i = 0; i < arrayLen; i++) {
      /* fetch the scalar located at i from the array.*/
      pvalue = av_fetch(ret,i,0);
      /* dereference the scalar into a numeric value. */
      retval = SvNV(*pvalue);
      Inline_Stack_Push(newSVnv(retval));
    }
  }
  else {
    Inline_Stack_Push(newSVnv(0));
  }
  Inline_Stack_Done;
}
END_OF_PERCENT2_C_CODE

[download]

I fixed the memory leak by creating and pushing new SVs into data_tmp and ret. And then using av_undef.
This memory leak fix is slightly slower because of the newSVnv lines.

Updated code snippets: 
    /* use new tmp array to prevent reordering caller array*/
    for (i = 0; i < arrayLen; i++) {
      pvalue = av_fetch(data,i,0);
      /* get double, create new SV, and push it into data_tmp array */
      retval = SvNV(*pvalue);
      av_push(data_tmp,newSVnv(retval));
    }

               ...

        /* fetch scalar located at calculated index*/
        pvalue = av_fetch(data_tmp,index,0);
        /* get double, create new SV, and push it into return array */
        retval = SvNV(*pvalue);
        av_push(ret,newSVnv(retval));

              ...

  Inline_Stack_Done;
  /* free memory */
  av_undef(data_tmp);
  av_undef(ret);

[download]
My questions are:

1) In the original code with the mem leak, when I tried only adding "av_undef(data_tmp)" to the very end, it erased @data
and I got the error "Attempt to free unreferenced scalar".
Why? Can you explain this?
I was expecting the reference count for each value in @array to decrease from 2 to 1...not go to 0

2) Is there a more efficient way to do this without altering the order of @data?

Thanks!

In reply to Inline C memory leak by skateamac

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":


  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.