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 Markus Wiederkehr <ma...@gmail.com> on 2008/12/14 21:35:23 UTC

[mime4j] open issues MIME4J-66 and 67

Sorry about my last comment on MIME4J-66. I did not realize that it is
about Base64Encoder, not Base64OutputStream..

But is Base64Encoder really necessary? I mean
CodecUtil.encodeBase64(InputStream, OutputStream) could also be
implemented as:
        Base64OutputStream b64Out = new Base64OutputStream(out);
        copy(in, b64Out);
        b64Out.close();

Why maintain two versions?

I also noticed that CodecUtil.encodeBase64 is not called anywhere in
Mime4j. Could we remove it entirely?

The same arguments apply to QuotedPrintableEncoder and
CodecUtil.encodeQuotedPrintable..

Markus

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Re: [mime4j] open issues MIME4J-66 and 67

Posted by Stefano Bagnara <ap...@bago.org>.
Robert Burrell Donkin ha scritto:
> On Sun, Dec 14, 2008 at 8:35 PM, Markus Wiederkehr
> <ma...@gmail.com> wrote:
>> Sorry about my last comment on MIME4J-66. I did not realize that it is
>> about Base64Encoder, not Base64OutputStream..
>>
>> But is Base64Encoder really necessary? I mean
>> CodecUtil.encodeBase64(InputStream, OutputStream) could also be
>> implemented as:
>>        Base64OutputStream b64Out = new Base64OutputStream(out);
>>        copy(in, b64Out);
>>        b64Out.close();
>>
>> Why maintain two versions?
> 
> copy uses more memory and is slower

"encoder.encode" does nothing different than "copy".

encode reads a byte[] buffer from IN, encode it in another byte[] and
then write it to OUT.

the outputstream based copy instead:

reads a byte[] buffer from IN, calls the write, write encode it in
another byte[] and then writes to OUT.

The bytes are copied the same number of times.

>> I also noticed that CodecUtil.encodeBase64 is not called anywhere in
>> Mime4j. Could we remove it entirely?
>>
>> The same arguments apply to QuotedPrintableEncoder and
>> CodecUtil.encodeQuotedPrintable..

For QP it's not the same. QuotedPrintableOutputStream  and
CodecUtil.encodeQuotedPrintable use the same underlying encoder to do
the encoding (QuotedPrintableEncoder). Instead the Base64 encoding is
present as 2 different implementations.

> these methods are more performant but less well tested. take them out
> if you like.

While architecturally speaking it *may* be true, this is FALSE for the
current implementation. At least my last outputstream was already faster
than the encoder. Markus say that the new outputstream is twice faster
than my outputstream, so it should be a lot faster than the encoder.

Unless we start using directly byte[] buffers if we use inputstreams and
outputstreams there is no difference in the needed copy between the 2
patterns.

Stefano

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Re: [mime4j] open issues MIME4J-66 and 67

Posted by Norman Maurer <no...@apache.org>.
2008/12/15 Robert Burrell Donkin <ro...@gmail.com>:
> On Mon, Dec 15, 2008 at 12:33 PM, Markus Wiederkehr
> <ma...@gmail.com> wrote:
>> On Sun, Dec 14, 2008 at 11:49 PM, Robert Burrell Donkin
>> <ro...@gmail.com> wrote:
>>> On Sun, Dec 14, 2008 at 8:35 PM, Markus Wiederkehr
>>> <ma...@gmail.com> wrote:
>>>> Sorry about my last comment on MIME4J-66. I did not realize that it is
>>>> about Base64Encoder, not Base64OutputStream..
>>>>
>>>> But is Base64Encoder really necessary? I mean
>>>> CodecUtil.encodeBase64(InputStream, OutputStream) could also be
>>>> implemented as:
>>>>        Base64OutputStream b64Out = new Base64OutputStream(out);
>>>>        copy(in, b64Out);
>>>>        b64Out.close();
>>>>
>>>> Why maintain two versions?
>>>
>>> copy uses more memory and is slower
>>
>> I have written another performance test for this. The current code has
>> a throughput of about 6 mb/sec on my machine. The change to
>> Base64OutputStream I proposed boosts it up to 110 mb/sec..
>>
>> Plus, the current code does not even pass a simple roundtrip test
>> because it writes padding characters in the middle of the stream.
>>
>> I have attached my performance tests to MIME4J-71 if you want to take
>> a look at them.
>>
>> I suggest we remove Base64Encoder and close MIME4J-66 and -67.
>
> fine by me
>
> - robert

+1
Norman

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Re: [mime4j] open issues MIME4J-66 and 67

Posted by Robert Burrell Donkin <ro...@gmail.com>.
On Mon, Dec 15, 2008 at 12:33 PM, Markus Wiederkehr
<ma...@gmail.com> wrote:
> On Sun, Dec 14, 2008 at 11:49 PM, Robert Burrell Donkin
> <ro...@gmail.com> wrote:
>> On Sun, Dec 14, 2008 at 8:35 PM, Markus Wiederkehr
>> <ma...@gmail.com> wrote:
>>> Sorry about my last comment on MIME4J-66. I did not realize that it is
>>> about Base64Encoder, not Base64OutputStream..
>>>
>>> But is Base64Encoder really necessary? I mean
>>> CodecUtil.encodeBase64(InputStream, OutputStream) could also be
>>> implemented as:
>>>        Base64OutputStream b64Out = new Base64OutputStream(out);
>>>        copy(in, b64Out);
>>>        b64Out.close();
>>>
>>> Why maintain two versions?
>>
>> copy uses more memory and is slower
>
> I have written another performance test for this. The current code has
> a throughput of about 6 mb/sec on my machine. The change to
> Base64OutputStream I proposed boosts it up to 110 mb/sec..
>
> Plus, the current code does not even pass a simple roundtrip test
> because it writes padding characters in the middle of the stream.
>
> I have attached my performance tests to MIME4J-71 if you want to take
> a look at them.
>
> I suggest we remove Base64Encoder and close MIME4J-66 and -67.

fine by me

- robert

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Re: [mime4j] open issues MIME4J-66 and 67

Posted by Markus Wiederkehr <ma...@gmail.com>.
On Mon, Dec 15, 2008 at 1:38 PM, Stefano Bagnara <ap...@bago.org> wrote:
> Markus Wiederkehr ha scritto:
>> On Sun, Dec 14, 2008 at 11:49 PM, Robert Burrell Donkin
>> <ro...@gmail.com> wrote:
>>> On Sun, Dec 14, 2008 at 8:35 PM, Markus Wiederkehr
>>> <ma...@gmail.com> wrote:
>>>> Sorry about my last comment on MIME4J-66. I did not realize that it is
>>>> about Base64Encoder, not Base64OutputStream..
>>>>
>>>> But is Base64Encoder really necessary? I mean
>>>> CodecUtil.encodeBase64(InputStream, OutputStream) could also be
>>>> implemented as:
>>>>        Base64OutputStream b64Out = new Base64OutputStream(out);
>>>>        copy(in, b64Out);
>>>>        b64Out.close();
>>>>
>>>> Why maintain two versions?
>>> copy uses more memory and is slower
>>
>> I have written another performance test for this. The current code has
>> a throughput of about 6 mb/sec on my machine. The change to
>> Base64OutputStream I proposed boosts it up to 110 mb/sec..
>
> WOW! what do you mean by "current code" ? The Base64Encoder or the
> previous OutputStream?

The Base64Encoder that is currently used by CodecUtil.encodeBase64.
The previous Base64OutputStream had about 50 mb/sec.

> AFAIK CodecUtil.encodeBase64 is "published" but *not* used by mime4j, right?

Exactly.

Markus

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Re: [mime4j] open issues MIME4J-66 and 67

Posted by Stefano Bagnara <ap...@bago.org>.
Markus Wiederkehr ha scritto:
> On Sun, Dec 14, 2008 at 11:49 PM, Robert Burrell Donkin
> <ro...@gmail.com> wrote:
>> On Sun, Dec 14, 2008 at 8:35 PM, Markus Wiederkehr
>> <ma...@gmail.com> wrote:
>>> Sorry about my last comment on MIME4J-66. I did not realize that it is
>>> about Base64Encoder, not Base64OutputStream..
>>>
>>> But is Base64Encoder really necessary? I mean
>>> CodecUtil.encodeBase64(InputStream, OutputStream) could also be
>>> implemented as:
>>>        Base64OutputStream b64Out = new Base64OutputStream(out);
>>>        copy(in, b64Out);
>>>        b64Out.close();
>>>
>>> Why maintain two versions?
>> copy uses more memory and is slower
> 
> I have written another performance test for this. The current code has
> a throughput of about 6 mb/sec on my machine. The change to
> Base64OutputStream I proposed boosts it up to 110 mb/sec..

WOW! what do you mean by "current code" ? The Base64Encoder or the
previous OutputStream?

AFAIK CodecUtil.encodeBase64 is "published" but *not* used by mime4j, right?

> Plus, the current code does not even pass a simple roundtrip test
> because it writes padding characters in the middle of the stream.

I guess this is because of the known bug in base64Encoder
(https://issues.apache.org/jira/browse/MIME4J-67)

> I have attached my performance tests to MIME4J-71 if you want to take
> a look at them.
> 
> I suggest we remove Base64Encoder and close MIME4J-66 and -67.

+1

Stefano

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Re: [mime4j] open issues MIME4J-66 and 67

Posted by Markus Wiederkehr <ma...@gmail.com>.
On Sun, Dec 14, 2008 at 11:49 PM, Robert Burrell Donkin
<ro...@gmail.com> wrote:
> On Sun, Dec 14, 2008 at 8:35 PM, Markus Wiederkehr
> <ma...@gmail.com> wrote:
>> Sorry about my last comment on MIME4J-66. I did not realize that it is
>> about Base64Encoder, not Base64OutputStream..
>>
>> But is Base64Encoder really necessary? I mean
>> CodecUtil.encodeBase64(InputStream, OutputStream) could also be
>> implemented as:
>>        Base64OutputStream b64Out = new Base64OutputStream(out);
>>        copy(in, b64Out);
>>        b64Out.close();
>>
>> Why maintain two versions?
>
> copy uses more memory and is slower

I have written another performance test for this. The current code has
a throughput of about 6 mb/sec on my machine. The change to
Base64OutputStream I proposed boosts it up to 110 mb/sec..

Plus, the current code does not even pass a simple roundtrip test
because it writes padding characters in the middle of the stream.

I have attached my performance tests to MIME4J-71 if you want to take
a look at them.

I suggest we remove Base64Encoder and close MIME4J-66 and -67.

Markus

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Re: [mime4j] open issues MIME4J-66 and 67

Posted by Robert Burrell Donkin <ro...@gmail.com>.
On Sun, Dec 14, 2008 at 8:35 PM, Markus Wiederkehr
<ma...@gmail.com> wrote:
> Sorry about my last comment on MIME4J-66. I did not realize that it is
> about Base64Encoder, not Base64OutputStream..
>
> But is Base64Encoder really necessary? I mean
> CodecUtil.encodeBase64(InputStream, OutputStream) could also be
> implemented as:
>        Base64OutputStream b64Out = new Base64OutputStream(out);
>        copy(in, b64Out);
>        b64Out.close();
>
> Why maintain two versions?

copy uses more memory and is slower

> I also noticed that CodecUtil.encodeBase64 is not called anywhere in
> Mime4j. Could we remove it entirely?
>
> The same arguments apply to QuotedPrintableEncoder and
> CodecUtil.encodeQuotedPrintable..

these methods are more performant but less well tested. take them out
if you like.

- robert

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org