eclipse-ee4j / angus-mail

Angus Mail
https://eclipse-ee4j.github.io/angus-mail
Other
53 stars 16 forks source link

POP3Message.writeTo(OutputStream os); writes each byte individually to the OutputStream #99

Open DanskerDave opened 1 year ago

DanskerDave commented 1 year ago

org.eclipse.angus.mail.pop3.POP3Message.writeTo(OutputStream os); writes each byte individually to the OutputStream

If you precede it with... org.eclipse.angus.mail.pop3.POP3Message.getInputStream().transferTo(OutputStream os); ...both will use a byte[] Buffer.

Attached (see later) is an example. (it uses test.mailu.io which seems to be a publicly accessible Demo Mail Server)

Look for the TODO & switch the order of readMessageBytesRaw(...) & readMessageBytesRFC822(...) to reproduce.

I would expect it to buffer its writes, rather than writing each byte individually.

Desktop:

Mail server:

Java 17 Example:

package angus.mail.test.bytewrite;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.Properties;

import org.eclipse.angus.mail.pop3.POP3Folder;
import org.eclipse.angus.mail.pop3.POP3Message;
import org.eclipse.angus.mail.pop3.POP3SSLProvider;
import org.eclipse.angus.mail.pop3.POP3Store;

import jakarta.mail.Folder;
import jakarta.mail.MessagingException;
import jakarta.mail.NoSuchProviderException;
import jakarta.mail.Session;

public class ReadTestByteWrite {

    private static final record ReadResult (byte[] bytes, int byteWrites, int arrayWrites) {}

    private static final class MyBAOS extends ByteArrayOutputStream {

        public int byteWrites  = 0;
        public int arrayWrites = 0;

        @Override
        public synchronized void reset() {
            super               .reset();

            byteWrites  = 0;
            arrayWrites = 0;
        }
        @Override
        public synchronized void write(final int b) {
            super               .write(          b);

            byteWrites++;
        }
        @Override
        public synchronized void write(final byte b[], final int off, final int len) {
            super               .write(           b,             off,           len);

            arrayWrites++;
        }
    }

    private static final int    PORT_995_POP_SSL_TLS = 995;

    private static final String PROTOCOL_POP3S       = "pop3s";
    private static final String PROTOCOL             = "protocol";
    private static final String MAIL                 = "mail";
    private static final String STORE                = "store";
    private static final String HOST                 = "host";
    private static final String PORT                 = "port";

    private static String dotJoin(final CharSequence... elements) {
        return String.join(".", elements);
    }

    public static void main(final String[] args) throws NoSuchProviderException, MessagingException, IOException {

        final var host     = "test.mailu.io";
        final var port     = String.valueOf(PORT_995_POP_SSL_TLS);

        final var username = "admin@test.mailu.io";
        final var password = "letmein";

        final var props = new Properties();
        /**/      props.setProperty(dotJoin(MAIL, STORE,          PROTOCOL), PROTOCOL_POP3S);
        /**/      props.setProperty(dotJoin(MAIL, PROTOCOL_POP3S, HOST),     host);
        /**/      props.setProperty(dotJoin(MAIL, PROTOCOL_POP3S, PORT),     port);

        /**/      props.setProperty("mail.pop3.ssl.socketFactory.class",     javax.net.ssl.SSLSocketFactory.class.getName());
        /**/      props.setProperty("mail.pop3.ssl.socketFactory.port",      port);
        /**/      props.setProperty("mail.pop3.starttls.enable",             "true");

        final var emailSession = Session.getInstance(props);
        final var provider     = new POP3SSLProvider();

        /**/                          System.out.println("Provider..: "     + provider);
        props.forEach((key, value) -> System.out.println("Property..: key=" + key + '\t' + "value=" + value));
        /**/                          System.out.println("Session...: "     + emailSession);

        try (final var emailStore = emailSession.getStore())
        {
            System.out.println("Store.....: " + emailStore +  " class=" + emailStore.getClass());

            emailStore.connect(username, password);

            System.out.println("Connected.: " + emailStore);

            if (emailStore instanceof POP3Store pop3store) {
                processPOP3store(pop3store);
            } else {
                throw new IllegalStateException("Mailbox is not POP3.: " + emailStore.getClass());
            }
        }
    }

    private static void processPOP3store(final POP3Store pop3store) throws MessagingException, IOException {

        try (final Folder inboxFolder = pop3store.getFolder("INBOX"))
        {
            inboxFolder.open(Folder.READ_ONLY);

            System.out.println("Inbox.....: " + inboxFolder +  " type=" + inboxFolder.getType() +  " class=" + inboxFolder.getClass());

            if (inboxFolder instanceof POP3Folder pop3inboxFolder) {
                processPOP3inbox(pop3inboxFolder);
            } else {
                throw new IllegalStateException("Folder is not POP3 INBOX.: " + inboxFolder.getClass());
            }
        }
    }

    private static void processPOP3inbox(final POP3Folder pop3inboxFolder) throws MessagingException, IOException {

        System.out.println("POP3 Inbox: " + pop3inboxFolder +  " count=" + pop3inboxFolder.getMessageCount());

        final var baos = new MyBAOS(); // (reuse this ByteArrayOutputStream for ALL Messages)

        for (final var message : pop3inboxFolder.getMessages()) {

            if (message instanceof POP3Message pop3message) {
                processPOP3message(pop3inboxFolder.getUID(pop3message), pop3message, baos);
            } else {
                throw new IllegalStateException("Message is not POP3.: " + message.getClass());
            }
        }
    }

    private static void processPOP3message(final String uid, final POP3Message pop3message, final MyBAOS baos) throws IOException, MessagingException {

        final var mailSentDate = pop3message.getSentDate();

        /*
         * IMPORTANT.: FIRST read the Message as RAW, then read it as RFC 822
         * -> reason.: this way, the Buffer will be filled using byte-array writes for BOTH reads
         * ..........: (the other way round, the RFC 822 read will transfer byte-by-byte)
         * 
         * TODO REVERSE THE ORDER OF THE FOLLOWING 2 READS. The RFC822 read will then write each Byte individually to the OutputStream.
         */
        final var rRaw    = readMessageBytesRaw   (pop3message, baos);
        final var rRFC822 = readMessageBytesRFC822(pop3message, baos);

        System.out.println("Raw Msg...: id=" + uid +  " sent=" + mailSentDate +  " arrayWrites=" + rRaw   .arrayWrites() + " byteWrites=" + rRaw   .byteWrites +  " L=" + rRaw   .bytes().length);
        System.out.println("RFC822 Msg: id=" + uid +  " sent=" + mailSentDate +  " arrayWrites=" + rRFC822.arrayWrites() + " byteWrites=" + rRFC822.byteWrites +  " L=" + rRFC822.bytes().length);
        System.out.println();
    }

    private static ReadResult readMessageBytesRFC822(final POP3Message pop3message, final MyBAOS baos) throws IOException, MessagingException {
        /**/                                    baos.reset();
        pop3message                    .writeTo(baos);
        return                   new ReadResult(baos.toByteArray(), baos.byteWrites, baos.arrayWrites);
    }

    private static ReadResult readMessageBytesRaw   (final POP3Message pop3message, final MyBAOS baos) throws IOException, MessagingException {
        /**/                                    baos.reset();
        pop3message.getInputStream().transferTo(baos);
        return                   new ReadResult(baos.toByteArray(), baos.byteWrites, baos.arrayWrites);
    }
}
jmehrens commented 7 months ago

The benchmark should set both the host and the protocol host.

props.setProperty(dotJoin(MAIL, HOST), host);
props.setProperty(dotJoin(MAIL, PROTOCOL_POP3S, HOST),     host);

Setting the host skips reverse DNS lookups which can slow down the first benchmark result and get cached for the second result.

That said, we can't upgrade to use transferTo internally until we drop support for JDK 8. That is in the works though.

DanskerDave commented 5 months ago

Jason, thanks for your feedback: I've updated my Benchmark.

Good to hear JDK 8 is being phased out. (my advice to people using JDK 8 is "stop it !!" )

My tip for the next version would be JDK 17:

Until then, you could maybe inline those 11 lines.

    public long transferTo(OutputStream out) throws IOException {
        Objects.requireNonNull(out, "out");
        long transferred = 0;
        byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
        int read;
        while ((read = this.read(buffer, 0, DEFAULT_BUFFER_SIZE)) >= 0) {
            out.write(buffer, 0, read);
            transferred += read;
        }
        return transferred;
    }
jmehrens commented 5 months ago

@DanskerDave Thanks for the tip. Upgrading to minimum version of JDK11 will most likely happen in the current release. Once that happens I should be able to use transferTo.