On Tue, May 25, 2010 at 5:16 PM, Xavier Chantry <[email protected]> wrote: > On Tue, May 25, 2010 at 10:06 PM, Andres P <[email protected]> wrote: >> I was about to do that, but first I think I should talk about another >> change. >> >> `type -p foo` has a return val, so consider this: >> $ time for i in {1..1000}; do [[ $(type -p sh) ]]; done >> real 0m1.564s >> user 0m0.160s >> sys 0m0.337s >> >> $ time for i in {1..1000}; do type -p sh &>/dev/null; done >> real 0m0.166s >> user 0m0.060s >> sys 0m0.103s >> >> Even though there is a negligible perfomance difference in makepkg, this >> is the correct way to check for $?. >> > > I don't understand this sentence. > negligible = 10x ? > And if your "correct" way is 10x faster, why do you say "even though" ? > This makes it sound like we need to accept a compromise (like worse > perf) for a correct behavior. > But from my understanding of your mail, you are saying that using type > -p directly is both more correct and faster. > I'm saying negligible *in* makepkg. I'm making it very clear that the justification for a change is not performance. Obviously, the same scale on that timediff could be applied to makepkg, but even then, the gains from implementing my suggestion are unperceivable.
>> If the redir to /dev/null becomes to much of a burden to type, it could >> become a function. Not to mention that the whole series of checking >> for vcs in PATH, which is where most of `type p` gets used, could be >> changed in to a more platable chunk. >> >> Would there be interest in making this change? >> >> I really think that throwing away the return val and making a string >> check is poor practice. >> >> > > So the only downside of using type -p sh &>/dev/null directly without > [/[[ and a subshell is that we need to type &>/dev/null ? > It does not seem like a big deal to me. > And yeah if it's used a lot, like more than 5 times, just make a function. > Just to be absolutely clear, I prefer &>/dev/null over the current way of handling this. This is why I proposed this. ;) -- Andres P
