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: [sqlite] SQLite kind-of memory leak (PATCH) - bug reports
Clifford Wolf <[EMAIL PROTECTED]> wrote: > > > but instead start a ticket at: > > http://www.sqlite.org/cvstrac/tktnew > > what shall I do with the patch? simply copy it to the 'bug description' > text field? i haven't seen a file upload function in the interface. > There is an "[attach]" hyperlink on the upper right of the ticket viewer page. -- 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: [sqlite] SQLite kind-of memory leak (PATCH) - bug reports
Hi, thanks for the quick reply, On Mon, Oct 03, 2005 at 08:35:10AM -0400, Griggs, Donald wrote: > Page http://www.sqlite.org/support.html Suggests that you *not* directly > email the (actually pretty responsive) author, the page says: Please do not send email directly to the author of SQLite unless [..] You are working on an open source project. and there are plenty of open source projects with my name on them, so I thought it would be ok. > but instead start a ticket at: > http://www.sqlite.org/cvstrac/tktnew what shall I do with the patch? simply copy it to the 'bug description' text field? i haven't seen a file upload function in the interface. yours, - clifford -- L I N : B I T ___ ___ __ _ |_ | The OSS cluster synchronization tool for / __(_-http://oss.linbit.com/csync2/ ] --- /___/ "This 'telephone' has too many shortcomings to be seriously considered as a means of communication." - Western Union internal memo, 1876.
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: [sqlite] SQLite kind-of memory leak (PATCH) - bug reports
Hi Clifford,
Page http://www.sqlite.org/support.html
Suggests that you *not* directly email the (actually pretty responsive)
author, but instead start a ticket at:
http://www.sqlite.org/cvstrac/tktnew
Donald Griggs
Opinions are not necessarily those of Misys Healthcare Systems nor its board
of directors.
-Original Message-
From: Clifford Wolf [mailto:[EMAIL PROTECTED]
Sent: Monday, October 03, 2005 8:00 AM
To: SQLite Users
Cc: [EMAIL PROTECTED]
Subject: [sqlite] SQLite kind-of memory leak (PATCH)
Hi,
I have sent the following mail to [EMAIL PROTECTED] about a month ago. But I got
no reply and as far as I can see my patch has not been applied in the sqlite
cvs so far.
If [EMAIL PROTECTED] is the wrong address to send sqlite development issues,
what is the right one? I can only find a pointer to this 'sqlite users'
list on the webpage.
yours,
- clifford
On Mon, Sep 12, 2005 at 01:04:13PM +0200, Clifford Wolf wrote:
> Hi,
>
> I'm the developer of a scripting language called SPL. This scripting
> language also has a module for accessing sqlite databases.
>
> 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..
>
> yours,
> - clifford
--snip--
Clifford Wolf:
Fixed kind-of-memory-leak in sqlite-3.2.0 for unix platforms
(This is not really a memory leak - just unfreed memory which
is confusing valgrind and other leak checkers..)
--- sqlite-3.2.0/src/os_unix.c 2005-09-09 23:21:57.0 +0200
+++ sqlite-3.2.0/src/os_unix.c 2005-09-10 22:38:29.0 +0200
@@ -231,6 +231,9 @@
static Hash lockHash = { SQLITE_HASH_BINARY, 0, 0, 0, 0, 0 }; static Hash
openHash = { SQLITE_HASH_BINARY, 0, 0, 0, 0, 0 };
+static int lockHashCounter = 0;
+static int openHashCounter = 0;
+
#ifdef SQLITE_UNIX_THREADS
/*
@@ -302,6 +305,8 @@
pLock->nRef--;
if( pLock->nRef==0 ){
sqlite3HashInsert(, >key, sizeof(pLock->key), 0);
+if (--lockHashCounter == 0)
+ sqlite3HashClear();
sqliteFree(pLock);
}
}
@@ -313,6 +318,8 @@
pOpen->nRef--;
if( pOpen->nRef==0 ){
sqlite3HashInsert(, >key, sizeof(pOpen->key), 0);
+if (--openHashCounter == 0)
+ sqlite3HashClear();
sqliteFree(pOpen->aPending);
sqliteFree(pOpen);
}
@@ -360,6 +367,7 @@
pLock->cnt = 0;
pLock->locktype = 0;
pOld = sqlite3HashInsert(, >key, sizeof(key1), pLock);
+lockHashCounter++;
if( pOld!=0 ){
assert( pOld==pLock );
sqliteFree(pLock);
@@ -383,6 +391,7 @@
pOpen->nPending = 0;
pOpen->aPending = 0;
pOld = sqlite3HashInsert(, >key, sizeof(key2), pOpen);
+openHashCounter++;
if( pOld!=0 ){
assert( pOld==pOpen );
sqliteFree(pOpen);
--snap--
--
L The SPL Programming Language
SP P L http://www.clifford.at/spl/
L
S PL An object oriented, stateful, simple, small, c-like,
P embeddable, feature rich, dynamic scripting language
Qrpelcgvat ebg13 ivbyngrf gur QZPN! Cercner gb or fhrq!!
