On Wed, Mar 18, 2015 at 12:59:18AM -0700, Alan Irwin wrote: > @Andrew: There are some really interesting release implications here, > and if you feel Norman's fix (ideally in git am form as I have > requested from him below) or some other variant of it you like is a > no-brainer please go ahead and push it (see my further comment on the > release delay implications at the end.).
Norman, I'm not sure I agree with your patch here in general. I've tested it on Linux and it makes no difference to my valgrind results. Further, adding in some debugging statements shows that at the point you set plsc->tidy the dispatch table entry is NULL anyway so your code makes no difference. This is with dynamic drivers. Are you using static drivers? In that case things may be different and we need to investigate more closely. Also, looking at the code in plP_tidy, the tidy function dispatch_table[dev - 1]->pl_tidy should already be called. Again debugging statements show this is the case, so I can well see why you end up with a double call for the psc driver if your change was having an effect. Can you confirm the build options you are using for plplot, and also the precise way you ran valgrind to generate this feedback. Since I'm unable to reproduce your results I'm a bit confused at precisely what the memory issue is. Thanks Andrew > > On 2015-03-17 20:28-0700 Norman Goldstein wrote: > > > I noticed that at the end of plSelectDev(), in plcore.c, > > a couple more fields of the PLStream > > needed to be filled in (feedback from valgrind :-)), > > so I added these two lines at line 3230: > > > > plsc->tidy = (void*) dispatch_table[dev - 1]->pl_tidy; > > plsc->tidy_data = (void*) plsc; > > > > I expect the first line is correct for all devices. > > I think the 2nd line is OK, too (I checked it is correct for > > XWindows, the only device I am using) > > > > Hi Norm: > > As you are probably already aware, I plan to release a new version of > PLplot soon based on the git master tip version so your test of that > version (see access directions for master tip at > https://sourceforge.net/p/plplot/plplot/ci/master/tree/) would be much > appreciated. > > Just to make sure we are on the same page, here is what I get for > master tip for dev psc > (using the CMake option -DBUILD_TEST=ON and following up with "make > all" to build all the necessary prerequisites first. > > software@raven> valgrind examples/c/x00c -dev psc -o test.psc > ==9610== Memcheck, a memory error detector > ==9610== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al. > ==9610== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info > ==9610== Command: examples/c/x00c -dev psc -o test.psc > ==9610== > ==9610== > ==9610== HEAP SUMMARY: > ==9610== in use at exit: 0 bytes in 0 blocks > ==9610== total heap usage: 470 allocs, 470 frees, 132,943 bytes allocated > ==9610== > ==9610== All heap blocks were freed -- no leaks are possible > ==9610== > ==9610== For counts of detected and suppressed errors, rerun with: -v > ==9610== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4) > > i.e., perfect valgrind results. > > And I also get perfect valgrind results for > software@raven> valgrind examples/c/x00c -dev svg -o test.svg > > Note that both psc and svg are non-interactive devices that have no > external library dependencies. > > In contrast, here are the -dev xwin results for the same (simple) example. > > ==9758== Memcheck, a memory error detector > ==9758== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al. > ==9758== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info > ==9758== Command: examples/c/x00c -dev xwin > ==9758== > ==9758== > ==9758== HEAP SUMMARY: > ==9758== in use at exit: 42,535 bytes in 444 blocks > ==9758== total heap usage: 1,442 allocs, 998 frees, 434,710 bytes allocated > ==9758== > ==9758== LEAK SUMMARY: > ==9758== definitely lost: 88 bytes in 1 blocks > ==9758== indirectly lost: 58 bytes in 2 blocks > ==9758== possibly lost: 0 bytes in 0 blocks > ==9758== still reachable: 42,389 bytes in 441 blocks > ==9758== suppressed: 0 bytes in 0 blocks > ==9758== Rerun with --leak-check=full to see details of leaked memory > ==9758== > ==9758== For counts of detected and suppressed errors, rerun with: -v > ==9758== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4) > > Do you also confirm this -dev xwin leak issue for master tip? > > Assuming your answer is yes, does your above fix turn this into a > perfect valgrind result (again for master tip) without compromising > the perfect -dev svg and -dev psc results? > > Also, if you have what you feel is a definitive fix, and you also have > some git knowledge could you send it here using the "git am" command > so we know exactly the change you want to make and so we can give you > proper credit for your work? > > On the other hand, If you don't have a lot of git experience, we are > still happy to except your suggestion for a fix! Furthermore, if for > this case you decide you would like to gain some git experience, then > I would highly recommend you start by reading our README.developers > file (for master tip) which gives useful hints for our particular git > workflow. We wrote that up from the perspective of git newbies (which > most of our active developers [including me] were when we started to > use git for Plplot development last summer). > > @Andrew: > > Although I am really anxious to get out the release, also note that if > Norman or you (based on his suggestions) find a definitive fix for > this particular plend-related issue for -dev xwin without compromising > our perfect -dev psc and -dev psc results, then I would be willing to > delay our release to test his fix (or your modification of same) > extensively for all our noninteractive and interactive devices. Also, > I assume you would be interested in doing such tests on your platforms > as well. The fact is we currently have a lot of plend-related issues > for many of our interactive devices right now (and even a few > noninteractive devices) so if there is a fix that cleans up a > significant fraction of those issues for this release without causing > other problems, it would make me a happy man! > > Alan > __________________________ > Alan W. Irwin > > Astronomical research affiliation with Department of Physics and Astronomy, > University of Victoria (astrowww.phys.uvic.ca). > > Programming affiliations with the FreeEOS equation-of-state > implementation for stellar interiors (freeeos.sf.net); the Time > Ephemerides project (timeephem.sf.net); PLplot scientific plotting > software package (plplot.sf.net); the libLASi project > (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); > and the Linux Brochure Project (lbproject.sf.net). > __________________________ > > Linux-powered Science > __________________________ > > ------------------------------------------------------------------------------ > Dive into the World of Parallel Programming The Go Parallel Website, sponsored > by Intel and developed in partnership with Slashdot Media, is your hub for all > things parallel software development, from weekly thought leadership blogs to > news, videos, case studies, tutorials and more. Take a look and join the > conversation now. http://goparallel.sourceforge.net/ > _______________________________________________ > Plplot-devel mailing list > Plplot-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/plplot-devel ------------------------------------------------------------------------------ Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ _______________________________________________ Plplot-devel mailing list Plplot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/plplot-devel
