Jeff King <peff@xxxxxxxx> writes: > On Thu, Jan 14, 2021 at 12:06:20PM -0500, Taylor Blau wrote: > >> > > + * This function runs in time O(log N) with the number of objects in the pack. >> > >> > Is it a good idea to commit to such performance characteristics as a >> > promise to callers like this (the comment applies to all three >> > functions)? >> > >> > It depends on how a developer is helped by this comment when >> > deciding whether to use this function, or find other ways, to >> > implement what s/he wants to do. >> >> I don't mind it. If they all had the same performance characteristics, I >> wouldn't be for it, but since they don't, I think that it's good to >> know. Peff suggested this back in [1]. > > Yeah, I asked for this. As somebody who has frequently worked on the > code which accesses the revindex (mostly bitmap stuff), I found it > useful to understand how expensive the operations were. However, I also > know what their runtimes are at this point, and it is not like somebody > interested cannot look at the implementation. So it may not be that > important. > > So I do still think it is useful, but if somebody feels strongly against > it, I don't mind it being removed. That won't be me. It's not like you'd use pack_pos_to_index() combined with pack_pos_to_offset() instead of offset_to_pack_pos() because the latter is more expensive than using the other two functions; the comment does not help those who want to know relative performance of these functions for such a purpose. I was just curious who the comments were meant to help, that's all. Thanks.
