You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by "Noel J. Bergman" <no...@devtech.com> on 2002/10/27 19:14:01 UTC
NNTPSpooler bug
Would someone please sanity check me on this, please?
Here is the output from heap checking after 36 hours:
percent live alloc'ed stack class
rank self accum bytes objs bytes objs trace name
1 3.76% 3.76% 151312 3153 11322240 235880 7960 java.lang.String
That is the #1 entry by live bytes. 3153 strings totaling ~150K. Checking
the stack trace, I see:
TRACE 7960:
java.io.UnixFileSystem.list(UnixFileSystem.java:Native method)
java.io.File.list(File.java:914)
org.apache.james.nntpserver.repository.NNTPSpooler$SpoolerRunnable.run(NNTPS
pooler.java:207)
java.lang.Thread.run(Thread.java:536)
This is kind of interesting, since other than leaving it enabled, I'm not
using NNTP. Checking the code ...
public void run() {
getLogger().debug("in spool thread");
try {
while ( Thread.currentThread().interrupted() == false ) {
String[] list = spoolPath.list();
for ( int i = 0 ; i < list.length ; i++ ) {
getLogger().debug("Files to process: "+list.length);
if ( lock.lock(list[i]) ) {
File f = new File(spoolPath,list[i]).getAbsoluteFile();
getLogger().debug("Processing file:
"+f.getAbsolutePath());
try {
process(f);
} catch(Exception ex) {
getLogger().debug("Exception occured while
processing file: " +
f.getAbsolutePath(),ex);
} finally {
lock.unlock(list[i]);
}
}
}
// this is good for other non idle threads
try {
Thread.currentThread().sleep(threadIdleTime);
} catch(InterruptedException ex) {
// Ignore and continue
}
}
} finally {
Thread.currentThread().interrupted();
}
}
Amongst other things, during the entire sleep time, we are keeping a lot of
strings locked in memory. Seems to me that at the end of the for() loop, we
should add:
list[i] = null; // release this string entry
and after the for() loop:
list = null; // release the array.
I have some other questions, but this is the first thing that jumped out.
--- Noel
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: NNTPSpooler bug
Posted by "Noel J. Bergman" <no...@devtech.com>.
> Spooler thread reads input articles, processes(moves to
> appropriate group) and them deletes them.
Understood.
> String[] list = spoolPath.list();
> This would usu. return list of length 0.
Understood. I thought it quite odd that I would have so much memory
committed to those strings, especially since I don't have NNTP active. But
I am at INFO, so I don't have any useful data. Will toggle that for my next
test.
> Seems sensible and harmless.
That's what I figured, too.
> I would hold of till 2.1 is done
I'll keep it as a private patch until post-2.1. It isn't critical, since
the space eventually gets released. But I do want to see if I can find out
why so much of it was in use, given that I don't HAVE any NNTP articles!
--- Noel
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: NNTPSpooler bug
Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
> Would someone please sanity check me on this, please?
>
Spooler thread reads input articles, processes(moves to appropriate group)
and them deletes them.
String[] list = spoolPath.list();
This would usu. return list of length 0.
If list.length > 0
this line should give useful information
L209 getLogger().debug("Files to process:
"+list.length);
If you are not using nntp, list.length would always be 0 and you will not
see this debug message.
Are you seeing this debug statement ?
> Amongst other things, during the entire sleep time, we are keeping a lot
of
> strings locked in memory. Seems to me that at the end of the for() loop,
we
> should add:
>
> list[i] = null; // release this string entry
>
> and after the for() loop:
>
> list = null; // release the array.
Seems sensible and harmless.
At present gc on list can only occur after the sleep cycle. This would allow
gc to occur in the sleep cycle.
If you change this (I would hold of till 2.1 is done) it may also be a good
idea to swap line L208 and L209
Harmeet
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: NNTPSpooler bug
Posted by "Noel J. Bergman" <no...@devtech.com>.
> can we push this back 'til the next cycle?
Sure. It isn't a memory leak, so much as a delay in releasing memory.
--- Noel
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: NNTPSpooler bug
Posted by Danny Angus <da...@apache.org>.
can we push this back 'til the next cycle? or is this tabled for this release already..
> -----Original Message-----
> From: Noel J. Bergman [mailto:noel@devtech.com]
> Sent: 27 October 2002 18:14
> To: James-Dev Mailing List
> Subject: NNTPSpooler bug
>
>
> Would someone please sanity check me on this, please?
>
> Here is the output from heap checking after 36 hours:
>
> percent live alloc'ed stack class
> rank self accum bytes objs bytes objs trace name
> 1 3.76% 3.76% 151312 3153 11322240 235880 7960 java.lang.String
>
> That is the #1 entry by live bytes. 3153 strings totaling ~150K.
> Checking
> the stack trace, I see:
>
> TRACE 7960:
> java.io.UnixFileSystem.list(UnixFileSystem.java:Native method)
> java.io.File.list(File.java:914)
>
> org.apache.james.nntpserver.repository.NNTPSpooler$SpoolerRunnable
> .run(NNTPS
> pooler.java:207)
> java.lang.Thread.run(Thread.java:536)
>
> This is kind of interesting, since other than leaving it enabled, I'm not
> using NNTP. Checking the code ...
>
> public void run() {
> getLogger().debug("in spool thread");
> try {
> while ( Thread.currentThread().interrupted() == false ) {
> String[] list = spoolPath.list();
> for ( int i = 0 ; i < list.length ; i++ ) {
> getLogger().debug("Files to process: "+list.length);
> if ( lock.lock(list[i]) ) {
> File f = new
> File(spoolPath,list[i]).getAbsoluteFile();
> getLogger().debug("Processing file:
> "+f.getAbsolutePath());
> try {
> process(f);
> } catch(Exception ex) {
> getLogger().debug("Exception occured while
> processing file: " +
> f.getAbsolutePath(),ex);
> } finally {
> lock.unlock(list[i]);
> }
> }
> }
> // this is good for other non idle threads
> try {
> Thread.currentThread().sleep(threadIdleTime);
> } catch(InterruptedException ex) {
> // Ignore and continue
> }
> }
> } finally {
> Thread.currentThread().interrupted();
> }
> }
>
> Amongst other things, during the entire sleep time, we are
> keeping a lot of
> strings locked in memory. Seems to me that at the end of the
> for() loop, we
> should add:
>
> list[i] = null; // release this string entry
>
> and after the for() loop:
>
> list = null; // release the array.
>
> I have some other questions, but this is the first thing that jumped out.
>
> --- Noel
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>