On 09/26/2014 09:00 PM, Junio C Hamano wrote: > Michael Haggerty <[email protected]> writes: > >> Even the one lockfile object needn't be allocated each time the >> function is called. Instead, define one statically-allocated >> lock_file object and reuse it for every call. >> >> Suggested-by: Jeff King <[email protected]> >> Signed-off-by: Michael Haggerty <[email protected]> >> --- >> ... >> - hold_locked_index(lock, 1); >> + hold_locked_index(&lock, 1); >> refresh_cache(REFRESH_QUIET); >> if (active_cache_changed && >> - write_locked_index(&the_index, lock, COMMIT_LOCK)) >> + write_locked_index(&the_index, &lock, COMMIT_LOCK)) > > I wondered if the next step would be to lose the "lock" parameter > from {hold,write}_locked_index() and have them work on a > process-global lock, but that would not work well. > > The reason why this patch works is because we are only working with > a single destination (i.e. $GIT_INDEX_FILE typically .git/index), > right? > > Interesting.
Ummm, this patch wasn't supposed to be interesting. If it is then maybe I made a mistake... My reasoning was that after the lock is acquired, it is released unconditionally before the function exits. Therefore, it should be no problem to reuse it each time the function is called. Of course, one gap in this argument is the possibility that this function calls itself recursively. The fact that it calls merge_recursive() should have alerted me to this possibility. So let me check... * try_merge_strategy() is only called by cmd_merge() * cmd_merge() is only called by the command dispatcher So I don't see a way for this function to call itself recursively within a single process. A second possible gap in this argument would be if this function can be called from multiple threads. But hardly any of our code is thread-safe, so I hardly think that is likely. What am I missing? Michael -- Michael Haggerty [email protected] -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
