Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

speed - imap use optimization #31

Closed
tomholub opened this issue Jun 7, 2017 · 9 comments
Closed

speed - imap use optimization #31

tomholub opened this issue Jun 7, 2017 · 9 comments
Assignees

Comments

@tomholub
Copy link
Collaborator

tomholub commented Jun 7, 2017

We have to do some speed improvements. I'm currently also working on a Desktop app for Windows/Linux/Macos that also uses IMAP, so I have a little better understanding of this now.

Dealing with folders:

  • folders should be cached. When I open it first time, it should load the list of folders and save them somewhere local. Next time I open it, it should:
  • render the folders from the cache
  • pull messages from active folder (typically inbox)
  • when idle, re-fetch the list of folders

This will mean we don't have to wait extra time while the app is pulling list of folders every time.

Displaying messages: loading a message takes way too long. I suspect we are creating a new connection for each screen?

  • we should be keeping and re-using a single connection throughout the app, because IMAP login is an expensive operation
  • because of this, we should be able to detect a broken/failed connection and automatically reconnect it as needed before making further requests
  • in the future, we may even want to create a special connection to download attachments and do similar work. That way, the app doesn't get stuck/blocked while an attachment is being downloaded.
@tomholub tomholub added this to the Usability milestone Jun 7, 2017
@DenBond7
Copy link
Collaborator

DenBond7 commented Jun 7, 2017

It's a very interesting assignment. And it's not easy :)

Displaying messages: loading a message takes way too long. I suspect we are creating a new connection for each screen?

Yes, for now for the prototype we are creating a new connection for each screen.

This task will take a long time, and I will have to investigate some moments.

@tomholub
Copy link
Collaborator Author

tomholub commented Jun 7, 2017

Yes I understand that. It makes sense, I would do the same for a prototype. But the responsiveness/usefulness of the app would really suffer if we left it this way, I think.

@DenBond7
Copy link
Collaborator

DenBond7 commented Jun 7, 2017

Yes, we need to make improvements that you described. Then the application will be much more comfortable.

@tomholub
Copy link
Collaborator Author

@DenBond7 somehow it was failing during build. Maybe some of your updates to Folder class did not get committed? The commit above is just temporary so that I can work with the project.

@DenBond7
Copy link
Collaborator

Maybe some of your updates to Folder class did not get committed?

@tomholub Yes, that's right. I missed one file for the commit.

@DenBond7
Copy link
Collaborator

For me

Admittedly the thread safety rules for JavaMail are not well documented, but hopefully they mostly match what you would expect.

Multiple threads can use a Session.

Since a Transport represents a connection to a mail server, and only a single thread can use the connection at a time, a Transport will synchronize access from multiple threads to maintain thread safety, but you'll really only want to use it from a single thread.

Similarly, a Store can be used by multiple threads, but access to the underlying connection will be synchronized and single threaded.

A Message should only be modified by a single thread at a time, but multiple threads should be able to read a message safely (although it's not clear why you would want to do that).

https://stackoverflow.com/questions/12732584/threadsafety-in-javamail

DenBond7 added a commit that referenced this issue Jun 16, 2017
Created EmailSyncService, GmailSynsManager and other.
DenBond7 added a commit that referenced this issue Jun 21, 2017
…SyncService. | #31.

Added a "messages" table to the database.
DenBond7 added a commit that referenced this issue Jun 23, 2017
DenBond7 added a commit that referenced this issue Jun 24, 2017
DenBond7 added a commit that referenced this issue Jun 24, 2017
@tomholub
Copy link
Collaborator Author

I like the progress here! Let me know when it's ready to test, I'm looking forward to try it.

DenBond7 added a commit that referenced this issue Jul 1, 2017
…Added handle errors from EmailSyncService. | #31.
DenBond7 added a commit that referenced this issue Jul 3, 2017
DenBond7 added a commit that referenced this issue Jul 3, 2017
@DenBond7
Copy link
Collaborator

DenBond7 commented Jul 4, 2017

Finally, I made the stable version for testing. Now you can start testing.

After the work done, I would like to add some comments:

  1. At the moment, the key object in synchronization is the EmailSyncService. Thanks to him, we use a single connection for all requests.

  2. I compared how different mail clients work. For the study, I took Gmail and BlueMail. For more logical work, I think we need to improve the way we send messages and moving them to other folders. For example, we'd better mark messages as deleted and delete them in the background, like in Gmail. (Similarly sending, replying and archiving)

  3. Thanks to the first, I prepared the ground for the implementation of correct synchronization. The current implementation does not take into account many moments

  • We equally use all types of connections - WIFI, 3G and others. This can be bad for the device's autonomy.
  • The automatic loading of new messages has not yet been implemented. You can only download them manually.
  • Also we do not track the battery level. This is also not very good.

@tomholub
Copy link
Collaborator Author

Perfect, I like that you prepared it for auto sync. That will be great in the future. I'm closing this for now unless there are things that you want to add now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants