On Tue, May 10, 2016 at 02:30:11PM -0500, Derek Martin wrote:
> On Mon, May 09, 2016 at 08:57:55PM -0400, Damien Riegel wrote:
> > On Mon, May 09, 2016 at 06:42:33PM -0500, Derek Martin wrote:
> > > On Sat, May 07, 2016 at 04:14:36AM -0400, Damien Riegel wrote:
> > > OK, I get it now.  I think this could work OK if the parent did a
> > > single stat, and perhaps did an opendir if the object was a directory,
> > > and then passed copies/pointers/whatever of those objects into the
> > > registered probe functions (pointers for sure).  There's no need to do
> > > multiple stats.  Then at least the only efficiency loss is the extra
> > > function calls.  If dir, the probe function should assume it needs to
> > > do rewinddir().
> >
> > Thanks for your input Derek. My code was based on the assumption that
> > mx_get_magic is idempotent, but maybe it is not true. 
> 
> Now that I've actually looked at the code... ;-)
> 
> I think it is, in the sense that you mean.  That's not the issue
> though, if I understand correctly.

I was just saying that before addressing the performance concern, we
needed to be sure that the code could actually work when mx_get_magic is
called multiple times.

Overall, I agree with you, calling mx_get_magic n times is inefficient,
at least more inefficient than calling it once! I thought the tradeoff
was worth it, but I did not really consider network storage at that
time.

> 
> The issue is that Mutt is going to call your registered probe
> functions one at a time, until one of them returns that it matches the
> mailbox type.  Currently in all cases this is a function that wraps
> the mx_get_magic() function, which always checks every possible path,
> until it finds one that matches.

Yes, this was a first implementation to keep simple, and avoid breaking
up mx_get_magic in different parts while it's still used in other places.

> ...
> 
> Does this make sense?

I see what you're getting at, but I think it is very complex for what it
is actually doing. I don't mean it is useless, just that it looks like
(too) early optimization to me. By that, I mean that if someone is using
mutt over a slow network, chances are that it is already bearly usable.
I would rather start with something simple and optimize it later if
there is a need for it.

If an implementation with some kind of cache is really (really really)
wanted, I think caching the result of stat, and eventually keeping the
file descriptor open if path points to a file; should be enough.

> Note also that you need not necessarily implement something like this
> for the first pass...  It's of course up to Kevin how and when (and
> if) he wants to take the various patches.

As I told you in a previous email, I prefered to get rid of the probing
logic altogether. It caused the most friction and it can be done later,
at the very end of the mailbox decoupling. We will re-discuss
implementation details when the time comes, but for now, I'd rater focus
on the v2 of this pathset and adding more functions in mx_ops.

Thanks,
-- 
Damien

Reply via email to