GNOME Bugzilla – Bug 397130
Camel's IMAP provider first copies to the memory before writing it to the cache
Last modified: 2013-08-12 11:24:25 UTC
Please describe the problem: This is unnecessary, the patch that I will attach to this bug prevents this extra copy and therefore reduces memory (peak) usage (peak because it does indeed get freed up, but nevertheless is the memory needlessly allocated at some point in time). Steps to reproduce: 1. Delete the cache of a message from the CamelImapMessageCache 2. Get the same message (preferably a huge message with for example a big attachment mime part) 3. Measure memory consumption Actual results: Memory consumption skyrockets (the memory is copied twice, which should be measurable if you know the expected total message size) Expected results: The message can and therefore probably should be immediately written to the CamelImapMessageCache backing store, which is perfectly doable with the CamelStream API (so, why not?) Does this happen every time? Yup Other information: I will attach a patch
Created attachment 80358 [details] [review] Patch that overcomes this problem This patch uses a lower level API to achieve writing immediately to the CamelImapMessageCache backing store (to the CamelStream being a file stream). Please review.
Note that I had to rework this patch a little bit to remove a feature from camel-lite that is not expected to be in the normal Camel (yet): partial retrieval. Especially review the locking, as the if/then/else pieces caused the locking to be slightly different. Also note that the locks used to be recursive locks, this seems to have changed for some reason (and might have affected the functionality, a recursive lock is not the same as a normal lock). It shouldn't have affected it, I also did a few basic tests with Evolution which seemed to work. Nevertheless, do some extra testing ;)
this patch is a really gross hack... you shouldn't be mucking with tag values here nor manually parsing imap response data especially using camel_stream_buffer_gets... ugh. how can you possibly expect to only consume the specified literal length of data if you can't possibly know the amount of raw data you are reading?
I see the length problem and I think it can be rewritten with a normal stream on store->ostream too. It's just going to be a little bit more difficult. I did some tests with incorrectly formatted E-mails, and those too seemed to get written out correctly in the cache. But still, it's unclear code etc etc. For now I have fixed this in tinymail's camel with a rather large allocation on the heap. Taking the assumption that E-mails with lines larger than that #define are just broken anyway (most E-mails seem to be encoded to have lines < 80 columns). I was planning to implement support for BINARY at some point, so I guess then I will have to rewrite this piece again. Anyway. I understand that this current state is unacceptable (cause of the line limit) for Evolution's Camel.
It seems that in realistic situations, MIME messages are guaranteed to have lines <= 1000 characters in size, and that they don't contain '\0' (in case of using FETCH BODY, this is not so in case of using FETCH BINARY .. which isn't yet being used here). This means that indeed 512 is not right, but with 1000 it would be.
This is about the legacy IMAP provide which was removed in 3.8. Closing as OBSOLETE.