Quantcast

isoreg memory leak?

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

isoreg memory leak?

Simon Wotherspoon

I believe there is a memory leak in isoreg in the current version of R,
as I believe the following shows

 > gc()
          used (Mb) gc trigger (Mb) max used (Mb)
Ncells 120405  3.3     350000  9.4   350000  9.4
Vcells  78639  0.6     786432  6.0   392463  3.0
 > for(k in 1:100) {
+
+   y <- runif(10000)
+   isoreg(x,y)
+ }
 > rm(x)
 > rm(y)
 > gc()
          used (Mb) gc trigger (Mb) max used (Mb)
Ncells 121637  3.3     350000  9.4   350000  9.4
Vcells 578615  4.5    1300721 10.0  1300642 10.0
                ^^^


Looking at the C code, I believe the problem arises as a consequence of
using SETLENGTH to resize the result near the very end of isoreg.c,
and the solution is to make a copy of iKnots.


SEXP R_isoreg(SEXP y)
{
     int n = LENGTH(y), i, ip, known, n_ip;
     double tmp, slope;
     SEXP yc, yf, iKnots, ans;
     const char *anms[] = {"y", "yc", "yf", "iKnots", ""};

     /* unneeded: y = coerceVector(y, REALSXP); */

     PROTECT(ans = mkNamed(VECSXP, anms));

     SET_VECTOR_ELT(ans, 0, y = y);
     SET_VECTOR_ELT(ans, 1, yc = allocVector(REALSXP, n+1));
     SET_VECTOR_ELT(ans, 2, yf = allocVector(REALSXP, n));
     SET_VECTOR_ELT(ans, 3, iKnots= allocVector(INTSXP, n));

... calculation ...

     SETLENGTH(iKnots, n_ip);
     UNPROTECT(1);
     return(ans);
}


But if this is the problem, I am at a bit of a loss as to what SETLENGTH
is actually for in general.

Clearly my understanding of how allocation/gc works is a bit off here,
but I can't see how else the leak may occur.  Hope this is more use than
nuisance.

Simon.

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: isoreg memory leak?

Simon Urbanek

On Jan 16, 2011, at 10:41 PM, Simon Wotherspoon wrote:

>
> I believe there is a memory leak in isoreg in the current version of R, as I believe the following shows
>
> > gc()
>         used (Mb) gc trigger (Mb) max used (Mb)
> Ncells 120405  3.3     350000  9.4   350000  9.4
> Vcells  78639  0.6     786432  6.0   392463  3.0
> > for(k in 1:100) {
> +
> +   y <- runif(10000)
> +   isoreg(x,y)
> + }
> > rm(x)
> > rm(y)
> > gc()
>         used (Mb) gc trigger (Mb) max used (Mb)
> Ncells 121637  3.3     350000  9.4   350000  9.4
> Vcells 578615  4.5    1300721 10.0  1300642 10.0
>               ^^^
>
>
> Looking at the C code, I believe the problem arises as a consequence of using SETLENGTH to resize the result near the very end of isoreg.c,
> and the solution is to make a copy of iKnots.
>

AFAICS this is more of a reporting issue than a memory leak - ReleaseLargeFreeVectors() uses LENGTH() to determine the number by which to reduce R_LargeVallocSize - and it will be smaller than the actually released memory if SETLENGTH was used. I'm not sure how to fix it, because AFAICS the real size is not recorded anywhere (unless we use truelength for that but I'm not sure I know the implications). The good news is that it's not really a memory leak - the bad news is that the memory usage stats are rather an upper bound of the reality ;).

Cheers,
Simon



>
> SEXP R_isoreg(SEXP y)
> {
>    int n = LENGTH(y), i, ip, known, n_ip;
>    double tmp, slope;
>    SEXP yc, yf, iKnots, ans;
>    const char *anms[] = {"y", "yc", "yf", "iKnots", ""};
>
>    /* unneeded: y = coerceVector(y, REALSXP); */
>
>    PROTECT(ans = mkNamed(VECSXP, anms));
>
>    SET_VECTOR_ELT(ans, 0, y = y);
>    SET_VECTOR_ELT(ans, 1, yc = allocVector(REALSXP, n+1));
>    SET_VECTOR_ELT(ans, 2, yf = allocVector(REALSXP, n));
>    SET_VECTOR_ELT(ans, 3, iKnots= allocVector(INTSXP, n));
>
> ... calculation ...
>
>    SETLENGTH(iKnots, n_ip);
>    UNPROTECT(1);
>    return(ans);
> }
>
>
> But if this is the problem, I am at a bit of a loss as to what SETLENGTH is actually for in general.
>
> Clearly my understanding of how allocation/gc works is a bit off here, but I can't see how else the leak may occur.  Hope this is more use than nuisance.
>
> Simon.
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>
>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: isoreg memory leak?

luke-tierney
On Mon, 17 Jan 2011, Simon Urbanek wrote:

>
> On Jan 16, 2011, at 10:41 PM, Simon Wotherspoon wrote:
>
>>
>> I believe there is a memory leak in isoreg in the current version of R, as I believe the following shows
>>
>>> gc()
>>         used (Mb) gc trigger (Mb) max used (Mb)
>> Ncells 120405  3.3     350000  9.4   350000  9.4
>> Vcells  78639  0.6     786432  6.0   392463  3.0
>>> for(k in 1:100) {
>> +
>> +   y <- runif(10000)
>> +   isoreg(x,y)
>> + }
>>> rm(x)
>>> rm(y)
>>> gc()
>>         used (Mb) gc trigger (Mb) max used (Mb)
>> Ncells 121637  3.3     350000  9.4   350000  9.4
>> Vcells 578615  4.5    1300721 10.0  1300642 10.0
>>               ^^^
>>
>>
>> Looking at the C code, I believe the problem arises as a consequence of using SETLENGTH to resize the result near the very end of isoreg.c,
>> and the solution is to make a copy of iKnots.

Using SETLENGTH in this way is definitely not intended -- only code in
memory.c should use it (and for now in envir.c).  We should fix shis
use, and one other strange one in grevents.c, and probably should get
rid of it SETLENGTH altogether outside of memory.c.

Best,

luke

>>
>
> AFAICS this is more of a reporting issue than a memory leak - ReleaseLargeFreeVectors() uses LENGTH() to determine the number by which to reduce R_LargeVallocSize - and it will be smaller than the actually released memory if SETLENGTH was used. I'm not sure how to fix it, because AFAICS the real size is not recorded anywhere (unless we use truelength for that but I'm not sure I know the implications). The good news is that it's not really a memory leak - the bad news is that the memory usage stats are rather an upper bound of the reality ;).
>
> Cheers,
> Simon
>
>
>
>>
>> SEXP R_isoreg(SEXP y)
>> {
>>    int n = LENGTH(y), i, ip, known, n_ip;
>>    double tmp, slope;
>>    SEXP yc, yf, iKnots, ans;
>>    const char *anms[] = {"y", "yc", "yf", "iKnots", ""};
>>
>>    /* unneeded: y = coerceVector(y, REALSXP); */
>>
>>    PROTECT(ans = mkNamed(VECSXP, anms));
>>
>>    SET_VECTOR_ELT(ans, 0, y = y);
>>    SET_VECTOR_ELT(ans, 1, yc = allocVector(REALSXP, n+1));
>>    SET_VECTOR_ELT(ans, 2, yf = allocVector(REALSXP, n));
>>    SET_VECTOR_ELT(ans, 3, iKnots= allocVector(INTSXP, n));
>>
>> ... calculation ...
>>
>>    SETLENGTH(iKnots, n_ip);
>>    UNPROTECT(1);
>>    return(ans);
>> }
>>
>>
>> But if this is the problem, I am at a bit of a loss as to what SETLENGTH is actually for in general.
>>
>> Clearly my understanding of how allocation/gc works is a bit off here, but I can't see how else the leak may occur.  Hope this is more use than nuisance.
>>
>> Simon.
>>
>> ______________________________________________
>> [hidden email] mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>>
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

--
Luke Tierney
Statistics and Actuarial Science
Ralph E. Wareham Professor of Mathematical Sciences
University of Iowa                  Phone:             319-335-3386
Department of Statistics and        Fax:               319-335-3017
    Actuarial Science
241 Schaeffer Hall                  email:      [hidden email]
Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Loading...