Re: RE: [sqlite] SQLite kind-of memory leak (PATCH) - bug reports

2005-10-04 Thread Clifford Wolf
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

2005-10-03 Thread drh
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

2005-10-03 Thread Thomas Briggs

   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

2005-10-03 Thread Clifford Wolf
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

2005-10-03 Thread drh
"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]>