git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 17/23] get_packed_ref_cache(): assume "packed-refs" won't change while locked

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
May 17, 2017, 12:05 UTC
Message-ID
<eedc8f85c8237515bc66584649b1e3f3d547049c.1495014840.git.mhagger@alum.mit.edu>
In-Reply-To
<cover.1495014840.git.mhagger@alum.mit.edu>

If we've got the "packed-refs" file locked, then it can't change; there's no need to keep calling `stat_validity_check()` on it.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs/files-backend.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c549a8d956..a910c44f38 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -354,13 +354,18 @@ static void files_ref_path(struct files_ref_store *refs,
 
 /*
  * Get the packed_ref_cache for the specified files_ref_store,
- * creating it if necessary.
+ * creating and populating it if it hasn't been read before or if the
+ * file has been changed (according to its `validity` field) since it
+ * was last read. On the other hand, if we hold the lock, then assume
+ * that the file hasn't been changed out from under us, so skip the
+ * extra `stat()` call in `stat_validity_check()`.
  */
 static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *refs)
 {
 	const char *packed_refs_file = files_packed_refs_path(refs);
 
 	if (refs->packed &&
+	    !is_lock_file_locked(&refs->packlock) &&
 	    !stat_validity_check(&refs->packed->validity, packed_refs_file))
 		clear_packed_ref_cache(refs);
 
@@ -1300,10 +1305,11 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
 			    flags, timeout_value) < 0)
 		return -1;
 	/*
-	 * Get the current packed-refs while holding the lock.  If the
-	 * packed-refs file has been modified since we last read it,
-	 * this will automatically invalidate the cache and re-read
-	 * the packed-refs file.
+	 * Get the current packed-refs while holding the lock. It is
+	 * important that we call `get_packed_ref_cache()` before
+	 * setting `packed_ref_cache->lock`, because otherwise the
+	 * former will see that the file is locked and assume that the
+	 * cache can't be stale.
 	 */
 	packed_ref_cache = get_packed_ref_cache(refs);
 	/* Increment the reference count to prevent it from being freed: */
-- 
2.11.0
Previous: Michael HaggertyNext: Stefan Beller
Message 43 of 73 in “Prepare to separate out a packed_ref_store”
  1. 00/23 Prepare to separate out a packed_ref_storeMichael Haggerty, May 17, 2017
  2. 01/23 t3600: clean up permissions test properlyMichael Haggerty, May 17, 2017
  3. Jeff KingMay 17, 2017
  4. Michael HaggertyMay 17, 2017
  5. Junio C HamanoMay 18, 2017
  6. Michael HaggertyMay 19, 2017
  7. 02/23 refs.h: clarify docstring for the ref_transaction_update()-related fnsMichael Haggerty, May 17, 2017
  8. Stefan BellerMay 17, 2017
  9. Junio C HamanoMay 18, 2017
  10. 03/23 ref_iterator_begin_fn(): fix docstringMichael Haggerty, May 17, 2017
  11. 05/23 refs_ref_iterator_begin(): don't check prefixes redundantlyMichael Haggerty, May 17, 2017
  12. Jeff KingMay 17, 2017
  13. Michael HaggertyMay 17, 2017
  14. 07/23 ref_store: take `logmsg` parameter when deleting referencesMichael Haggerty, May 17, 2017
  15. Jeff KingMay 17, 2017
  16. Michael HaggertyMay 17, 2017
  17. Jeff KingMay 17, 2017
  18. 04/23 prefix_ref_iterator: don't trim too muchMichael Haggerty, May 17, 2017
  19. Jeff KingMay 17, 2017
  20. Michael HaggertyMay 17, 2017
  21. Jeff KingMay 17, 2017
  22. Junio C HamanoMay 18, 2017
  23. Michael HaggertyMay 18, 2017
  24. 09/23 files-backend: move `lock` member to `files_ref_store`Michael Haggerty, May 17, 2017
  25. Jeff KingMay 17, 2017
  26. Michael HaggertyMay 17, 2017
  27. 06/23 refs: use `size_t` indexes when iterating over ref transaction updatesMichael Haggerty, May 17, 2017
  28. Stefan BellerMay 17, 2017
  29. Michael HaggertyMay 18, 2017
  30. 10/23 files_ref_store: put the packed files lock directly in this structMichael Haggerty, May 17, 2017
  31. Jeff KingMay 17, 2017
  32. Michael HaggertyMay 17, 2017
  33. Stefan BellerMay 17, 2017
  34. Brandon WilliamsMay 18, 2017
  35. Michael HaggertyMay 19, 2017
  36. Brandon WilliamsMay 18, 2017
  37. Jeff KingMay 18, 2017
  38. Brandon WilliamsMay 18, 2017
  39. 12/23 ref_transaction_commit(): break into multiple functionsMichael Haggerty, May 17, 2017
  40. Stefan BellerMay 17, 2017
  41. Michael HaggertyMay 19, 2017
  42. 16/23 should_pack_ref(): new function, extracted from `files_pack_refs()`Michael Haggerty, May 17, 2017
  43. 17/23 get_packed_ref_cache(): assume "packed-refs" won't change while lockedMichael Haggerty, May 17, 2017
  44. Stefan BellerMay 17, 2017
  45. Jeff KingMay 18, 2017
  46. Stefan BellerMay 18, 2017
  47. 18/23 read_packed_refs(): do more of the work of reading packed refsMichael Haggerty, May 17, 2017
  48. 19/23 read_packed_refs(): report unexpected fopen() failuresMichael Haggerty, May 17, 2017
  49. Jeff KingMay 17, 2017
  50. Michael HaggertyMay 17, 2017
  51. Junio C HamanoMay 18, 2017
  52. Jeff KingMay 18, 2017
  53. 15/23 ref_update_reject_duplicates(): add a sanity checkMichael Haggerty, May 17, 2017
  54. 20/23 refs_ref_iterator_begin(): handle `GIT_REF_PARANOIA`Michael Haggerty, May 17, 2017
  55. Jeff KingMay 17, 2017
  56. 22/23 ref-filter: limit traversal to prefixMichael Haggerty, May 17, 2017
  57. Jeff KingMay 17, 2017
  58. Michael HaggertyMay 19, 2017
  59. 14/23 ref_update_reject_duplicates(): use `size_t` rather than `int`Michael Haggerty, May 17, 2017
  60. 23/23 cache_ref_iterator_begin(): avoid priming unneeded directoriesMichael Haggerty, May 17, 2017
  61. 21/23 create_ref_entry(): remove `check_name` optionMichael Haggerty, May 17, 2017
  62. 08/23 lockfile: add a new method, is_lock_file_locked()Michael Haggerty, May 17, 2017
  63. Jeff KingMay 17, 2017
  64. 11/23 files_transaction_cleanup(): new helper functionMichael Haggerty, May 17, 2017
  65. Jeff KingMay 17, 2017
  66. Michael HaggertyMay 19, 2017
  67. Stefan BellerMay 17, 2017
  68. Michael HaggertyMay 19, 2017
  69. 13/23 ref_update_reject_duplicates(): expose function to whole refs moduleMichael Haggerty, May 17, 2017
  70. Jeff KingMay 17, 2017
  71. Stefan BellerMay 17, 2017
  72. Johannes SixtMay 18, 2017
  73. Jeff KingMay 18, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.