Re: RE: [sqlite] SQLite kind-of memory leak (PATCH) - bug reports
Hi, On Mon, Oct 03, 2005 at 11:16:12AM -0400, [EMAIL PROTECTED] wrote: > I also added code to deallocate the hash tables when their size > reaches zero. [...] Thanks a lot. I am sure this is worth the overhead and makes sqlite an even better piece of software. yours, - clifford -- _ _ _ Nerds on Air - Radio for Geeks _ __ ___ | \| |___ /_\ On 1st and 3rd Friday of the month / |/ /__ / _ | | .` / _ \/ _ \21:00-22:00 CET on Radio Orange // _ \/ __ | |_|\_\___/_/ \_\ http://www.clifford.at/noa//_/|_/\___/_/ |_| perl -le '$_=1;(1x$_)!~/^(11+)\1+$/&${_}while$_++<1000'|fmt
Re: RE: [sqlite] SQLite kind-of memory leak (PATCH) - bug reports
Clifford Wolf <[EMAIL PROTECTED]> wrote: > Hi, > > On Mon, Oct 03, 2005 at 09:02:38AM -0400, [EMAIL PROTECTED] wrote: > > You are right: this is not a real memory leak. > > [..] > > in fact, for a program which is eg. continously using mktemp() (or a > simmilar but not unsecure api) for creating temporary databases it is a > real memory leak, because the hash table will grow one entry for every > temporary database created. > I added a new regression test named manydb.test to prove that the above is not a problem. I also added code to deallocate the hash tables when their size reaches zero. This is pointless code that is there only to make valgrind happy. But I get complaints about valgrind frequently enough that I've grown weary of reading them. So now SQLite is a little bigger and a little slower, but at least valgrind doesn't complain anymore. -- D. Richard Hipp <[EMAIL PROTECTED]>
RE: RE: [sqlite] SQLite kind-of memory leak (PATCH) - bug reports
While I can understand your general sentiment, allowing minor problems like this to clutter the output from valgrind makes spotting the real errors amidst the noise more difficult. Eventually, when enough of these types of problems exist, valgrind stops being used altogether, because it's too time consuming to inspect the output. In short, I guess my point is that this kind of thing is another example of "broken window syndrome" - keeping the small things tidy makes everyone more likely to keep the big things tidy too. -Tom > -Original Message- > From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] > Sent: Monday, October 03, 2005 9:03 AM > To: sqlite-users@sqlite.org > Subject: Re: RE: [sqlite] SQLite kind-of memory leak (PATCH) > - bug reports > > "Griggs, Donald" <[EMAIL PROTECTED]> wrote: > > > > > > I'm using valgrind for checking for memory leaks in SPL. When > > > profiling scripts which do access SQLite databases, I've > found that > > > the lockHash and openHash data structures in os_unix.c > don't get freed. > > > > > > I wouldn't consider that a real memory leak, but it > doesn't look nice > > > in memory profilers such as valgrind. That's why I recommend the > > > attached patch. Please let me know how you think about it.. > > > > > > > You are right: this is not a real memory leak. > > I am disinclined to add code to SQLite that serves no purpose > other than to make the output of valgrind look better. valgrind > is a nice tool for tracking down memory allocation problems. > (SQLite uses a different mechanism, but that should not be > taken as a slight by valgrind.) But I do not believe that > valgrind should become an end in itself. I will certainly make > whatever changes to SQLite are necessary to fix *real* memory > leaks. I would even be willing to modify existing code to > better suit valgrind as long as it doesn't add complexity > or have a run-time cost. But the changes submitted do have > a run-time cost, and while that cost is very small (perhaps > even unmeasurable) it is finite. We have worked *very* hard > to remove such minor costs from SQLite and it would be a shame > to add them back, just so that the output of a diagnostic > tool could look nicer. > > -- > D. Richard Hipp <[EMAIL PROTECTED]> > >
Re: RE: [sqlite] SQLite kind-of memory leak (PATCH) - bug reports
Hi, On Mon, Oct 03, 2005 at 09:02:38AM -0400, [EMAIL PROTECTED] wrote: > You are right: this is not a real memory leak. > [..] in fact, for a program which is eg. continously using mktemp() (or a simmilar but not unsecure api) for creating temporary databases it is a real memory leak, because the hash table will grow one entry for every temporary database created. > [..] just so that the output of a diagnostic tool could look nicer. what about an sqlite3_cleanup() function which does such cleanups? then it would add no overhead at all to the library. Right now it is impossible for a host application to do such cleanups because the allocated memory is only referenced by static variables in os_unix.c. I could provide a patch which adds an sqlite3_cleanup() api. it would even possible to put that code path into a seperate elf section so it is not paged into the physical memory unless it actually is used by the application, if the incrased code size would be your next argument. but I think that this would be overkill. yours, - clifford -- ___ _ __ _ _ www.rocklinux.org | _ \ / _ \ / ___| |/ / | | (_)_ __ _ ___ __ | |_) | | | | | | ' / | | | | '_ \| | | \ \/ / | _ <| |_| | |___| . \ | |___| | | | | |_| |> < |_| \_\\___/ \|_|\_\ |_|_|_| |_|\__,_/_/\_\
Re: RE: [sqlite] SQLite kind-of memory leak (PATCH) - bug reports
"Griggs, Donald" <[EMAIL PROTECTED]> wrote: > > > > I'm using valgrind for checking for memory leaks in SPL. When > > profiling scripts which do access SQLite databases, I've found that > > the lockHash and openHash data structures in os_unix.c don't get freed. > > > > I wouldn't consider that a real memory leak, but it doesn't look nice > > in memory profilers such as valgrind. That's why I recommend the > > attached patch. Please let me know how you think about it.. > > > You are right: this is not a real memory leak. I am disinclined to add code to SQLite that serves no purpose other than to make the output of valgrind look better. valgrind is a nice tool for tracking down memory allocation problems. (SQLite uses a different mechanism, but that should not be taken as a slight by valgrind.) But I do not believe that valgrind should become an end in itself. I will certainly make whatever changes to SQLite are necessary to fix *real* memory leaks. I would even be willing to modify existing code to better suit valgrind as long as it doesn't add complexity or have a run-time cost. But the changes submitted do have a run-time cost, and while that cost is very small (perhaps even unmeasurable) it is finite. We have worked *very* hard to remove such minor costs from SQLite and it would be a shame to add them back, just so that the output of a diagnostic tool could look nicer. -- D. Richard Hipp <[EMAIL PROTECTED]>