Ofir Gross wrote:
Hi,
I share my thoughts:
I appreciate this. Sorry If it takes so much to reply but I've been busy
with the spool synchronization problems and with my day-job.
I'd like to keep the conversation on the dev-list because it is archived
for future reference and somebody else could eventually partecipate to
the brainstorming.
There is a problem with the constructor:
public MimeMessage(MimeMessage source) throws MessagingException {
[...]
source.writeTo(bos); <<----------Problem is here
[...]
Because it __always__ saves into a ByteArrayInputStream which is in
memory. The only way to avoid it is either __not__ to call that
constructor, or to subclass it like I did, in order to overide this
method. When I have overiden this constructor, I saved to a temporary
file instead of a byte array.
Ok, that's clear, we must remove calls to that constructor from our
code, and we should use new MimeMessage(Session s, InputStream is).
Furthermore we should pass a SharedInputStream to that constructor.
You suggested to use the existing repository instead, which is a good
idea. But two questions:
1. how do I locate the message in the repository form the MimeMessage
"source"?
I think this is not possible. You can get the source starting from the
Mail object in the repository but not viceversa.
Why do you need that?
2. Does every MimeMessage exist in the repository?
Not every mimemessage, because when we create a new mimemessage from
scratch (like the bounce mailet) we create the MimeMessage and after
that we send it to James that store it in the spool.
BTW most time when we deal with big messages we probably received them
from the SMTPServer and we already have them in streamrepositories or
mailrepositories.
When the SMTPServer receive a new message it currently create a new
MailImpl using the "public MailImpl(String name, MailAddress sender,
Collection recipients, InputStream messageIn)" costructor.
That constructor create a new MimeMessageSource using the InputStream
from the socket (new MimeMessageInputStreamSource(name, messageIn);)
MimeMessageInputStreamSource currently store the message in a .m64 file
and eventually provide the stream to the following users:
public synchronized InputStream getInputStream() throws IOException {
return new BufferedInputStream(new FileInputStream(file));
}
This is the first step.
Maybe we should start changing the MimeMessageInputStreamSource to
provide a Shared input stream?!?
Then, when a message is stored in a dbfile repository then the header is
stored in the body field of the repository db table while the body of
the message is stored in a streamrepository.
MimeMessageJDBCSource is the object that handle this behaviour. So every
time we read a message from the db repository we use this object.
We call this method of that object to get the InputStream:
---
public synchronized InputStream getInputStream() throws IOException
---
and it create a new SequenceInputStream using a ByteArrayInputStream of
the header and a the input stream provided by the streamrepository.get().
---
InputStream in = new ByteArrayInputStream(headers);
if (sr != null) {
in = new SequenceInputStream(in, sr.get(key));
}
---
The implementation of the streamrepository.get used is in
File_Persistent_Stream_Repository and you can see it uses a
---
final ResettableFileInputStream stream = new ResettableFileInputStream(
getFile( key ) );
---
Maybe we should work on the above files to always be able to provide
shared inputstreams.
The other constructor: "public MimeMessage(Session session, InputStream
is)" will be OK if the InputStream provided to it will be a
SharedInputStream, and that can be done by tracking down calls to this
constractor (by grep "new MimeMessage"), and wrapping up the provided
input stream with a SharedInputStream implementing wrapper. I did that
grep and looked at the results, and this looks possible.
The main one is on the MimeMessageWrapper.loadMessage()
----
in = source.getInputStream();
headers = loadHeaders(in);
ByteArrayInputStream headersIn = new
ByteArrayInputStream(headers.toByteArray());
in = new SequenceInputStream(headersIn, in);
message = new MimeMessage(session, in);
----
source is the MimeMessageJDBCSource we already seen before and we could
change it to provide a SharedInputStream (to avoid a copy to a new file)
You see a similar pattern as before: we create a new SequenceInputStream
from a ByteArrayInputStream and the stream provided by the source.
We should find a way to have a SharedInputStream at the end of all this
steps, without the need to copy the stream on a new file.
We could introduce your proposed wrapped only in a few mailet but we
should avoid using it in our core because we already have 2 wrappers
over the mimemessage and we can get better/cleaner result following the
path I describe in this mail.
Maybe if all InputStreams provided to that constructor will be
SharedInputStream, then all MimeMessage will have a SharedInputStream
implementing contentStream as well? If it is true, then the overiding
constructor of "MimeMessage(MimeMessage source)" could use that stream,
and it will not need to locate the message in the repository. But I am
not sure wether it is true, because the constructor "MimeMessage(Session
session)" don't touch content, or contentStream, and so if it is
constructed that way, it will not have either, and so there will be no
InputStream for it, in case it is sent this way to the
MimeMessage(MimeMessage source) constructor.
When the message is constructed from someone else using the new
MimeMessage(session) we have no power on how it is handled.
IMHO we should start optimizing our own operations then we could write a
few docs on how to write optimized mailets.
A further step would be to use streaming operations also for "db" only
repositories. We currently use blob operations that write and read full
bytearrays but most new dbs/jdbc drivers correctly supports the use of
streams for write and read operations of large contents. Here there
would be one more issue because they don't provide (obviously)
SharedInputStreams but their own InputStreams and I don't know how we
could implement it. We probably should use your proposed wrapper if we
need that, or otherwise create a wrapper over the previous InputStream
implementing the SharedInputStream and simply retreaving a new
InputStream at every call to newStream (returning, in turn, a new
SharedInputStream).
The worst part of this work is that it is very difficult to unit-test
memory issues: any idea on how to test it?
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
