Re: [async-mqtt5] Review questions

From: Marcelo Zimbres Silva (mzimbres_at_[hidden])
Date: 2024-10-20 10:36:35

>
> Throughout the library, we’ve been careful to ensure that "you don’t get
> for what you did’t pay," especially when it comes to performance. In terms
> of logging, this means we wanted to avoid any runtime overhead associated
> with logging if you choose not to use it, even simple `if` checks.

I strongly disagree with you here, in pretty much every scenario I can
think of in terms of IO an if check is costless, this includes: resolve,
connect, handshake, reads and writes. You can't even measure it since IO
operations are orders of magnitude more costly.

On the other hand, lack of logging results in a lot of frustration not only
for your users but also for the authors as bug reports won't contain enough
information for you to investigate the problem. There are just so many
things that can go wrong, freeze etc.

In the absence of a better solution, we recommend users add their own
> logging code directly into the async_mqtt5 source during development. It’s
> very far from ideal, but we couldn’t come up with a better approach.

I don't think it is reasonable to expect users to even know where to add
log lines in a 15k loc codebase (that uses callback to implement its
composed operations). This will result in a lot of frustration.

It would also only be possible as long as this library is kept header-only
and I think it shouldn't.

What you could do here is to introduce a BOOST_MQTT5_ENABLE_BOOST_LOG macro
and add the lines yourself to the code.

However, conceptually, the right solution is still unclear. Technically, we
> understand how it can be done, but the question is more about the intended
> design. I recall Richard Hodges once explaining the role of a bound
> executor to a handler, stating that "the final handler, as well as all
> intermediate handlers of the async operation, will be invoked using the
> bound executor." While the initiation code of the async function may not
> use the bound executor, once the internal code calls another internal async
> function, every subsequent async function, including the final handler,
> will be executed using the originally bound executor.

AFAICS this leads into inconsistency since async_read uses the
conn.async_run executor while the async_write will use the async_publish
executor. That is why I argue that the bound executor in async_publish
should only be used to deliver the completion while everything else runs on
the connection executor, not on the async_run bound executor, which should
also be only used to deliver the completion.

Marcelo