Menu
▾
▴
gnuplot-beta
|
From: Daniel J S. <dan...@ie...> - 2006-03-13 18:10:10
|
Last night I started a script "allinf.dem" consisting of load "all.dem" load "allinf.dem" run with the system command line: gnuplot allinf.dem < /dev/null What I found was that last night the minimum memory usage in the cycle for gnuplot/gplt_x11 was 6.4/5.9 MB, but this morning it was 12.2/5.9 MB which suggests. I can't think of why it should go up. Is there some better way to test for memory leaks? Dan |
|
From: Dave D. <dde...@es...> - 2006-03-13 19:51:42
|
Daniel J Sebald <dan...@ie...> writes: > Last night I started a script "allinf.dem" consisting of > > load "all.dem" > load "allinf.dem" > > run with the system command line: > > gnuplot allinf.dem < /dev/null > load makes a recursive call into the command parser. So you have set up an infinite recursion. If you attached a debugger, you'd probably see a very deep call stack. Yeah - if I have $ cat /tmp/pause pause 1 "hello" $ cat /tmp/recurse load "/tmp/pause" load "/tmp/recurse" Then I run it for a while, and then (gdb) where #0 0xdfb7a923 in _libc_sigsuspend () #1 0xdfb939f0 in _libc_usleep () #2 0xdfbca8ad in usleep () #3 0x80515fb in pause_command () at command.c:997 #4 0x8050831 in command () at command.c:511 #5 0x8050409 in do_line () at command.c:368 #6 0x8079032 in load_file (fp=0x811d010, name=0x8122f30 "/tmp/pause", can_do_args=0 '\000') at misc.c:267 #7 0x805143b in load_command () at command.c:871 #8 0x8050831 in command () at command.c:511 #9 0x8050409 in do_line () at command.c:368 #10 0x8079032 in load_file (fp=0x811d000, name=0x8122f18 "/tmp/recurse", can_do_args=0 '\000') at misc.c:267 #11 0x805143b in load_command () at command.c:871 [snip] #164 0x8050831 in command () at command.c:511 #165 0x8050409 in do_line () at command.c:368 #166 0x8079032 in load_file (fp=0x811cd90, name=0x8122b70 "/tmp/recurse", can_do_args=0 '\000') at misc.c:267 #167 0x805143b in load_command () at command.c:871 #168 0x8050831 in command () at command.c:511 #169 0x8050409 in do_line () at command.c:368 #170 0x8079032 in load_file (fp=0x811cd80, name=0x804714b "/tmp/recurse", can_do_args=0 '\000') at misc.c:267 #171 0x807fe59 in main (argc=1, argv=0x8046f98) at plot.c:622 Problem goes away if you spell it $ cat /tmp/recurse load '/tmp/pause' reread dd -- Dave Denholm <dde...@es...> http://www.esmertec.com |
|
From: Daniel J S. <dan...@ie...> - 2006-03-13 20:03:55
|
Dave Denholm wrote: > Problem goes away if you spell it > > $ cat /tmp/recurse > load '/tmp/pause' > reread All give that a try. Dan |
|
From: Daniel J S. <dan...@ie...> - 2006-03-13 20:05:48
|
Daniel J Sebald wrote: > All give that a try. I'LL!... typping two flast. |
|
From: Daniel J S. <dan...@ie...> - 2006-03-13 22:55:39
|
Dave Denholm wrote: > load makes a recursive call into the command parser. So you have set > up an infinite recursion. [snip] > > Problem goes away if you spell it > > $ cat /tmp/recurse > load '/tmp/pause' > reread I ran with that construct for a few hours. I'm still seeing memory very slowly increasing. Was your thought that the command parser or command memory is slowly increasing and that should account for the memory increase? Dan |
|
From: Daniel J S. <dan...@ie...> - 2006-03-15 03:11:18
|
Daniel J Sebald wrote:
> I ran with that construct for a few hours. I'm still seeing memory very
> slowly increasing.
I may have by chance found at least one source of a memory leak without even going to valgrind yet.
Here is a function in set.c:
/* default settings for palette */
void
reset_palette()
{
if (!enable_reset_palette) return;
sm_palette.colorMode = SMPAL_COLOR_MODE_RGB;
sm_palette.formulaR = 7; sm_palette.formulaG = 5;
sm_palette.formulaB = 15;
sm_palette.positive = SMPAL_POSITIVE;
sm_palette.ps_allcF = 0;
sm_palette.use_maxcolors = 0;
sm_palette.gradient_num = 0;
sm_palette.gradient = NULL;
sm_palette.cmodel = C_MODEL_RGB;
sm_palette.gamma = 1.5;
pm3d_last_set_palette_mode = SMPAL_COLOR_MODE_NONE;
}
Notice that sm_palette.gradient is simply set to NULL. But that variable is a memory pointer assigned as
if (sm_palette.gradient) {
free( sm_palette.gradient );
}
sm_palette.gradient = (gradient_struct*)
gp_alloc( actual_size*sizeof(gradient_struct), "pm3d gradient" );
So, there needs to be a "free()" as part of that reset_palette() routine just as with the above hunk of code. Palettes can be big, so that could easily chew up a lot of memory for some applications.
I'd also make the argument that the routines
set_palette_defined()
set_palette_file()
set_palette_function()
check_palette_grayscale()
reset_palette()
really don't need to be inside set.c and should go inside either color.c or getcolor.c. That would help get rid of the use of a global structure variable sm_palette.
Some improvement in organization would also be pointer-based routines like compare_palette(), copy_palette(), destroy_palette() which partially exist already. Rather than have sm_palette, maybe that would work better as a pointer. What is nice about that is its more suitable for object-orientation as proposed by Hans if ever gnuplot moves to keeping track of multiple plot contents (rather than just the current).
Dan
|
|
From: Ethan A M. <merritt@u.washington.edu> - 2006-03-15 04:51:27
|
On Tuesday 14 March 2006 07:19 pm, Daniel J Sebald wrote:
> I may have by chance found at least one source of a memory leak
> without even going to valgrind yet.
Yup.
> reset_palette()
> {
> if (!enable_reset_palette) return;
> sm_palette.colorMode = SMPAL_COLOR_MODE_RGB;
> sm_palette.formulaR = 7; sm_palette.formulaG = 5;
> sm_palette.formulaB = 15;
> sm_palette.positive = SMPAL_POSITIVE;
> sm_palette.ps_allcF = 0;
> sm_palette.use_maxcolors = 0;
> sm_palette.gradient_num = 0;
> sm_palette.gradient = NULL;
> sm_palette.cmodel = C_MODEL_RGB;
> sm_palette.gamma = 1.5;
> pm3d_last_set_palette_mode = SMPAL_COLOR_MODE_NONE;
> }
> Notice that sm_palette.gradient is simply set to NULL.
Yup.
Notice also that sm_palette.color is neither freed nor set
to NULL. I wonder if this contributes to your main problem
of incorrect palette comparisons.
--
Ethan A Merritt
Biomolecular Structure Center
University of Washington, Seattle 98195-7742
|
|
From: Daniel J S. <dan...@ie...> - 2006-03-15 05:43:41
|
Ethan A Merritt wrote: >>Notice that sm_palette.gradient is simply set to NULL. > > > Yup. > Notice also that sm_palette.color is neither freed nor set > to NULL. I wonder if this contributes to your main problem > of incorrect palette comparisons. Oh yeah; I didn't notice that. That too should be changed. Dan |
|
From: Daniel J S. <dan...@ie...> - 2006-03-16 06:19:41
Attachments:
x11-leak-djs-15mar2006.patch
|
OK, valgrind has isolated a couple leaks in the pipe works (patch attached):
==10516==
==10516== ERROR SUMMARY: 137 errors from 11 contexts (suppressed: 27 from 1)
==10516== malloc/free: in use at exit: 144724 bytes in 1160 blocks.
==10516== malloc/free: 177326 allocs, 176166 frees, 505192128 bytes allocated.
==10516== For counts of detected errors, rerun with: -v
==10516== searching for pointers to 1160 not-freed blocks.
==10516== checked 5955140 bytes.
==10516==
==10516==
==10516== 1029 bytes in 2 blocks are definitely lost in loss record 1 of 5
==10516== at 0x1B904A90: malloc (vg_replace_malloc.c:131)
==10516== by 0x804B568: gp_alloc (alloc.c:268)
==10516== by 0x80BB3D8: X11_args (x11.trm:291)
==10516== by 0x8090785: main (plot.c:380)
This one is happening at line 291 of x11.trm. This command:
xargv = (char **) gp_alloc(argc * sizeof(char *), "<xargv>");
uses xargv like a normal pointer (i.e., ++) and never attempts to free the memory within X11_args(). The patch makes xargv a local static variable and duplicates the pointer as p_xargv used as ++p_xargv, etc.
==10516==
==10516==
==10516== 1130 bytes in 76 blocks are definitely lost in loss record 2 of 5
==10516== at 0x1B904A90: malloc (vg_replace_malloc.c:131)
==10516== by 0x283AEF: strdup (in /lib/tls/libc-2.3.3.so)
==10516== by 0x805F403: push (eval.c:484)
==10516== by 0x805F5BB: execute_at (eval.c:587)
This is right before the bad line in question.
/* WARNING - This is a memory leak if the string is not later freed */
I'll write off line on this one.
Dan
|
