Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: [PATCH] compinit forks less



I like Dana's approach even better and am happy to adapt my patch
or let them.  It also a tad faster for me on an old Skylake CPU
(more like 30..40 not 230).  I suspect it varies from OS to OS and
CPU to CPU, but it also just looks cleaner.  It seems to basically
work, but I've only just tested it weakly for a couple minutes.

It bears mention that while my first here-string idea uses two temp
files, this new still uses one temp file, and a racy one at that (not
that I see any real attack by messing up bindkey introspection).

To see, you can just (on Linux)
    strace zsh -fc 'opt=( ${(s< >)${ bindkey \^i }} )'

This is because of how ${ } is implemented which surprised me when
I looked into it after Mikael's comment.  I think the relevant code
is the part of Src/subst.c near a comment beginning "Admittedly a
hack".  Essentially, ${ } re-writes to `>| tempFileByName { expr }`.
There is no O_EXCL flag or anything on the open since there is no
redirection syntax like `>` for that (at least that I'm aware of)
to lean on.

I imagine this must have been discussed long ago when ${ } got in,
but if we're not going to fix it before the release a single sentence
about using less-safe temp files in the feature list in NEWS might be
warranted or even conceivably the man page.  While compinit bindkey
probing is unlikely a problem, other ${ } uses may not be as lucky.
It also makes the comment in the source code in the definition of
gettempname no longer true - all callers do not use O_EXCL.

Anyway, FWIW, I also tried:
    local f=/dev/shm/z$RANDOM # yeah, yeah..just a demo
    bindkey '^i' > $f
    IFS=$' \t' read -A opt < $f
    zf_rm $f   # requires assuming user loads zsh/files
and that was the same time to within 15+-10 µs of Dana's.  So, I
suspect the perf savings is mostly just "one less pass".  The point
of mentioning this besides being a pre-${ } idea is then that is
likely also roughly the perf benefit of an array variant - although
just not worrying about parsing seems the bigger win there.


Messages sorted by: Reverse Date, Date, Thread, Author