KIMAP2

[Christian Mollekopf]

On Tue, Nov 15, 2016, at 09:56 PM, Daniel Vrátil wrote:
> Hi Christian,
> 

Hey Dan,

> On Tuesday, November 15, 2016 3:13:09 PM CET Christian Mollekopf wrote:
> > Hi,
> > 
> > I have largely completed the transition to KIMAP2 and will thus retire
> > KIMAP. Since I know you are probably continuing to use KIMAP at least
> > for some time, I made KIMAP2 completely coinstallable and namespaced
> > everything into KIMAP2.
> > However I will only be able to maintain KIMAP2, and not KIMAP. If you
> > want to keep KIMAP around (I assume you do), I suggest we release it
> > from a stable branch "kimap", as I would like to move master to kimap2.
> > If that causes large problems with the way things are released let's
> > discuss alternative solutions.
> 
> Since we are probably going to stick with KIMAP1 for some time in
> Akonadi/
> KMail, I think we would still prefer to be able to follow the regular
> release 
> workflow, i.e. stable branch, master devel, next stable branched from
> master 
> etc. I think it would only be harder for release management to remember
> to do 
> some special branching in case of KIMAP repository.
> 
> How about simply setting up kimap2.git repo with KIMAP2? Once/if we port 
> Akonadi/KMail to KIMAP2, we can simply retire the KIMAP1 repository.
> 
> If there are interesting bugfixes in KIMAP2 that we would consider
> backporting 
> to KIMAP1, we can always take care of it manually (we probably would no
> be 
> able to merge anyway).
> 
> It also feels a bit more logical to me: having two releases of two
> products 
> from a single repository would just lead to a lot of confusion...

Sounds good to me. We should try to refrain from adding too much to
KIMAP1 otherwise we'll end up with two projects instead of one, but I
have no objections if that helps to smoothen the process for everyone
involved. 

> > In any case, I suggest to move to KIMAP2 in the long run.
> > 
> > So, here's what's changed with KIMAP2:
> > 
> > * KIMAP2 is now threading free:
> > There no longer is a dedicated thread for each socket. The reasoning for
> > this is manyfold:
> >     * The threading strategy should be up to the application. The
> >     current design imposes additional threads in any case.
> >     * The threading resulted in lot's of subtle bugs and code-complexity
> >     overhead for the synchronization.
> >     * The whole design revolved around the idea that by running the
> >     parser in a thread the parser can be blocking and the network will
> >     always be the limiting factor. That assumption is not necessarily
> >     true and the parser starts to behave very badly if we get data
> >     faster than we can process it (The internal buffer will get large
> >     and a majority of time will be spent on memcopying during the trim()
> >     call).
> >     * This design doesn't allow the consumer to regulate the amount of
> >     data that enters the system. It reads as much into memory as it can,
> >     which again assumes the network is the slowest part.
> >     * During benchmarking of Sink the parser actually became the
> >     bottleneck of the imap resource (because of the pathological
> >     behaviour with memcopy in trim()).
> > All of this directly leads to the next point.
> > 
> > * We have now a non-blocking parser:
> >     * The parser operates with two buffers to emulate a ringbuffer =>
> >     fixed memory usage (except for some dynamic allocation for large
> >     literals).
> >     * The parser never reads more data than it can process => If we
> >     can't process fast enough the socket buffer will eventually fill up,
> >     resulting in the server eventually stopping to send more data. Which
> >     is how the network stack is supposed to work IMO.
> >     * We open up possibilities for new streaming API to directly stream
> >     i.e. attachments to disk.
> >     * This resulted in the parser mostly vanishing from benchmarks and
> >     memcpy to vanish entirely.
> 
> I did not look into the code, but is the parser decoupled from the socket 
> publicly? I.e. can application move the socket into a secondary thread
> and run the parser in the main thread, just feeding it raw data from the socket?
> Or  do I have to move the parser to secondary thread, and then have the jobs
> run in the main thread and receive data through queued connections?
> 

The parser is not decoupled from the socket publicly. The session
(+loginjob) handles
the connection establishment + reaction to connection teardown and is
thus coupled to the
socket implementation.

Running anything in a separate thread thus means having the session
(that wraps the socket),
in that thread and running the job in the same thread.

IMO if performance or non-blocking behavior are the aim you should do as
much work as possible in the thread,
and not just parsing, so I don't see the point of a thread running just
the parser.
Maybe I just don't understand what you have in mind though, what would
be the usecase?


> 
> > 
> > * We no longer depend on kio:
> >     * This was used for ktcpsocket only, and the only remaining benefit
> >     of that was the KSslCertificateManager integration. This can easily
> >     be readded externally by implementing the Session::sslErrors
> >     callback and the Session::ignoreErrors function, as well as
> >     QSslSOckets default ca certificate functions. (We may want to extend
> >     the API there a bit).
> >     * KIO has a whole slew of unrelated dependencies so this was an
> >     important step to make KIMAP2 more selfcontained.
> 
> So is there some factory-like API that would allow us to provide our own 
> KTcpSocket, falling back to QSslSocket by default? If not, would that be
> an acceptable change for you?

There is no such API. This would require an additional SSlSocket
abstraction layer to
hide the API differences between QSslSocket and KTcpSocket. Doable but I
don't understand why you'd want to do that if you can have all the
benefits that KTcpSocket  has (the KSslCertificateManager integration),
by just readding that externally.

Overall I hope we can avoid introducing that extra abstraction layer and
either:
* Give you what you need from KTcpSocket by allowing to easily build the
necessary integrations externally (on top of QSslSocket). As I said, I
think this is possible with minimal effort.
* or, Untie the whole connection setup from KIMAP, so it is completely
socket agnostic. This would be interesting I guess because it would open
doors to things like:
   * Have a trusted process that handles the authentication (signond).
   * Have the trusted process pass over the already authenticated socket
   to the process wanting to do some imap.
   * Have the untrusted process (imap resource), pass the socket to
   kimap so it can do some imap without ever having seen any sort of
   credentials.

Of course the latter strays further from the current design and is thus
a bit harder to implement.


> > 
> > * The login job has received an overhaul.
> >     * Removed the slightly confused EncryptionMode names that sometime
> >     mixed the excryption and the use of starttls (which are two largely
> >     unrelated things).
> >     * Cleaned up the login logic which was a very complex statemachine
> >     scattered accross different classes and is now only a complex
> >     statemachine scattered over fewer classes =(
> >     * Fixed a potential race-condition where we would send a CAPABILITY
> >     request before receiving the greeting, which some servers seem to
> >     handle anyways but some don't.
> >     * Removed the encryption negotation which is handled by QSslSocket
> >     according to the provided settings, and otherwise fails as it
> >     should.
> 
> Cool :-) The login part is indeed complicated. I still have a SASL2
> plugin 
> that implements Google's XOAUTH authentication mechanism (basicall OAuth
> via IMAP), should probably try to upstream that to both KIMAPs :-)
> 

That would indeed be interesting to see whether that fits into the
current login procedure,
or whether spawns some ideas for a better design to accommodate that as
well.

> > 
> > There are some things left to do left and right, including some
> > extensions that I'll likely implement (NOTIFY comes to mind), and some
> > API cleanup,  but in general not much is standing in the way of a first
> > release.
> > I have not yet decided on a release date as so far as only Kube depends
> > on it so far.
> 
> Again, this would be settled by having kimap2.git repo I think, then the 
> different release needs of Kube and KMail would be solved...

That may very well be true.

Thanks for your feedback!

Cheers,
Christian