HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Is there any way to make this JavaMail code faster?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisanymakewayjavamailfastercodethere

Problem

I'm developing an application that receives and manages e-mails from a server. I manage only XML files and organize, by sender, the e-mails that have XML attachments, while the others are deleted. I create a list of files and create them in the temporary folder. I have a feeling my JavaMail process is slow because it accesses every message. Can I make my process faster?

```
public static void main(String[] args) {

try {

Properties properties = System.getProperties();
properties.setProperty("mail.store.protocol", "imaps");
Session session = Session.getDefaultInstance(properties, null);
Store store = session.getStore("imaps");
store.connect("imap.gmail.com", "login", "password");

Folder inbox = store.getDefaultFolder();
inbox = inbox.getFolder("INBOX");

inbox.open(Folder.READ_WRITE);

if (EmailUtil.hasMessage(inbox)) {

List listMessages = new ArrayList<>(Arrays.asList(inbox.getMessages()));
Iterator iterator = listMessages.iterator();

while (iterator.hasNext()) {

Message message = iterator.next();

if (EmailUtil.hasNoAttachment(message.getContentType())) {
EmailUtil.deleteMessage(message);
iterator.remove();
}
}

if (EmailUtil.hasMessagesWithAttachment(listMessages)) {

Set setSender = new HashSet<>();
List listFiles = new ArrayList<>();

listMessages = getAttachmentFiles(listMessages, setSender, listFiles);

if (EmailUtil.hasXmlAttachments(listFiles)) {

Set setFolder = getFolderSet(store);

List listFolderName = getFoldersName(setFolder);

removeExistingFoldersInSet(setSender, listFolderName);

if (foldersNotExist(setSender)) {
createFolder(store, setSender);

Solution

I have a feeling your application is slow because it does a lot of network access ... ;-)

The amount of time in your code will be a very small fraction of the actual 'latency'. So, the question is not "How can we make your code faster?" but rather it is "How can we reduce the amount of network traffic?"

I am not very well versed with managing the performance of the javamail API. When I have used it the Mail servers have been local, and not really a factor in performance... But, without actually trying it myself, you should be using the fetch(...,...) method.

Also, there's no real need for the outside if (EmailUtil.hasMessages(inbox))....

Sometimes it is better to explain with code, rather than with blurb:

// clear out any delete-marked messages - no need to process them....
inbox.expunge();
// by convention, this should be a lightweight process...
// hopefully GMail does it right.
Message[] messages = inbox.getMessages();
if (messages.length > 0) {
    Message[] todelete = new Message[messages.length];
    deletecount = 0;
    // now, instead of doing a loop through all the messages,
    // do a bulk 'get' operation to get the fields we know we will need:
    FetchProfile profile = new FetchProfile();
    // add the headers we know we will need:
    profile.add(FetchProfile.Item.CONTENT_INFO);
    // this is a bulk operation that should get the content-type,
    // and other things.
    inbox.fetch(messages, profile);
    for (int m = 0; m  0) {
        todelete = Arrays.copyOf(todelete, deletecnt);
        Flags delflags = new Flags(Flags.Flag.DELETED);
        inbox.setFlags(todelete, delflags, true);
        inbox.expunge();
    }

}


From this you can get the idea of what you should do....

I realize I kept the Message[] as an array, and you have it as a List. I kept it as an array because it is the input mechanism for the bulk methods of the API... but, in fairness, I think you should keep using the List format (the iterator.remove() is useful). Then, convert the List to an array when you need it.

Also, look in to the FetchProfile object/method, you can add any header fields you want to it...

Also, you can filter out messages that do not meet certain criteria, and then fetch additional data for the next set of tests, and do it that way.

Bottom line is that you want to access the server as few times as possible, and when you access it, you should get as little data as you need, but, group all similar requests in to a sigle operation.....

Code Snippets

// clear out any delete-marked messages - no need to process them....
inbox.expunge();
// by convention, this should be a lightweight process...
// hopefully GMail does it right.
Message[] messages = inbox.getMessages();
if (messages.length > 0) {
    Message[] todelete = new Message[messages.length];
    deletecount = 0;
    // now, instead of doing a loop through all the messages,
    // do a bulk 'get' operation to get the fields we know we will need:
    FetchProfile profile = new FetchProfile();
    // add the headers we know we will need:
    profile.add(FetchProfile.Item.CONTENT_INFO);
    // this is a bulk operation that should get the content-type,
    // and other things.
    inbox.fetch(messages, profile);
    for (int m = 0; m < messages.length; m++) {
        if (EmailUtil.hasNoAttachment(messages[m].getContentType())) {
            todelete[deletecnt++] = messages[m];
            messages[m] = null;
        }
    }
    // OK, do a bulk delete operation
    if (deletecnt > 0) {
        todelete = Arrays.copyOf(todelete, deletecnt);
        Flags delflags = new Flags(Flags.Flag.DELETED);
        inbox.setFlags(todelete, delflags, true);
        inbox.expunge();
    }

}

Context

StackExchange Code Review Q#36878, answer score: 6

Revisions (0)

No revisions yet.