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

Re: [PATCH v2 2/4] diffcore-rename: complete find_basename_matches()

From
Derrick Stolee <stolee@gmail.com>
Date
Feb 9, 2021, 13:25 UTC
Message-ID
<b64e30ab-9cb3-ffee-ec5e-1b94529ce636@gmail.com>
In-Reply-To
<dcd0175229aa6fba576425e78875b95385acb58d.1612870326.git.gitgitgadget@gmail.com>
On 2/9/2021 6:32 AM, Elijah Newren via GitGitGadget wrote:
> +	/*
> +	 * When I checked, over 76% of file renames in linux just moved
Perhaps "In late 2020," instead of "When I checked".
Show 26 quoted lines
> +	 * files to a different directory but kept the same basename.  gcc
> +	 * did that with over 64% of renames, gecko did it with over 79%,
> +	 * and WebKit did it with over 89%.
> +	 *
> +	 * Therefore we can bypass the normal exhaustive NxM matrix
> +	 * comparison of similarities between all potential rename sources
> +	 * and destinations by instead using file basename as a hint, checking
> +	 * for similarity between files with the same basename, and if we
> +	 * find a pair that are sufficiently similar, record the rename
> +	 * pair and exclude those two from the NxM matrix.
> +	 *
> +	 * This *might* cause us to find a less than optimal pairing (if
> +	 * there is another file that we are even more similar to but has a
> +	 * different basename).  Given the huge performance advantage
> +	 * basename matching provides, and given the frequency with which
> +	 * people use the same basename in real world projects, that's a
> +	 * trade-off we are willing to accept when doing just rename
> +	 * detection.  However, if someone wants copy detection that
> +	 * implies they are willing to spend more cycles to find
> +	 * similarities between files, so it may be less likely that this
> +	 * heuristic is wanted.
> +	 */
> +
> +	int i, renames = 0;
>  	struct strintmap sources;
>  	struct strintmap dests; 
...
> +	 * copy detection.  find_basename_matches() is only used when detecting
> +	 * renames, not when detecting copies, so it'll only be used when a file
> +	 * only existed in the source.  Since we already know that the file
There are two "only"s in this sentence. Just awkward, not wrong.
Show 7 quoted lines
> +	 * won't be unmodified, there's no point checking for it; that's just a
> +	 * waste of resources.  So set skip_unmodified to 0 so that
> +	 * estimate_similarity() and prefetch() won't waste resources checking
> +	 * for something we already know is false.
> +	 */
> +	int skip_unmodified = 0;
> +
Show 11 quoted lines
> -	/* TODO: Make use of basenames source and destination basenames */
> +	/* Now look for basename matchups and do similarity estimation */
> +	for (i = 0; i < num_src; ++i) {
> +		char *filename = rename_src[i].p->one->path;
> +		char *base = NULL;
> +		intptr_t src_index;
> +		intptr_t dst_index;
> +
> +		/* Get the basename */
> +		base = strrchr(filename, '/');
> +		base = (base ? base+1 : filename);

Here is the third instance of this in the same function. At minimum we should extract a helper for you to consume.

Show 40 quoted lines
> +		/* Find out if this basename is unique among sources */
> +		src_index = strintmap_get(&sources, base);
> +		if (src_index == -1)
> +			continue; /* not a unique basename; skip it */
> +		assert(src_index == i);
> +
> +		if (strintmap_contains(&dests, base)) {
> +			struct diff_filespec *one, *two;
> +			int score;
> +
> +			/* Find out if this basename is unique among dests */
> +			dst_index = strintmap_get(&dests, base);
> +			if (dst_index == -1)
> +				continue; /* not a unique basename; skip it */
> +
> +			/* Ignore this dest if already used in a rename */
> +			if (rename_dst[dst_index].is_rename)
> +				continue; /* already used previously */
> +
> +			/* Estimate the similarity */
> +			one = rename_src[src_index].p->one;
> +			two = rename_dst[dst_index].p->two;
> +			score = estimate_similarity(options->repo, one, two,
> +						    minimum_score, skip_unmodified);
> +
> +			/* If sufficiently similar, record as rename pair */
> +			if (score < minimum_score)
> +				continue;
> +			record_rename_pair(dst_index, src_index, score);
> +			renames++;
> +
> +			/*
> +			 * Found a rename so don't need text anymore; if we
> +			 * didn't find a rename, the filespec_blob would get
> +			 * re-used when doing the matrix of comparisons.
> +			 */
> +			diff_free_filespec_blob(one);
> +			diff_free_filespec_blob(two);
> +		}
> +	}
Makes sense to me.

Thanks, -Stolee

Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren
Message 21 of 71 in “Optimization batch 7: use file basenames to guide rename detection”
  1. 0/3 Optimization batch 7: use file basenames to guide rename detectionElijah Newren via GitGitGadget, Feb 6, 2021
  2. 1/3 diffcore-rename: compute basenames of all source and dest candidatesElijah Newren via GitGitGadget, Feb 6, 2021
  3. 2/3 diffcore-rename: complete find_basename_matches()Elijah Newren via GitGitGadget, Feb 6, 2021
  4. 3/3 diffcore-rename: guide inexact rename detection based on basenamesElijah Newren via GitGitGadget, Feb 6, 2021
  5. Derrick StoleeFeb 7, 2021
  6. Junio C HamanoFeb 7, 2021
  7. Elijah NewrenFeb 8, 2021
  8. Derrick StoleeFeb 8, 2021
  9. Elijah NewrenFeb 8, 2021
  10. Junio C HamanoFeb 8, 2021
  11. Elijah NewrenFeb 8, 2021
  12. Junio C HamanoFeb 8, 2021
  13. Elijah NewrenFeb 8, 2021
  14. Elijah NewrenFeb 8, 2021
  15. Derrick StoleeFeb 8, 2021
  16. Elijah NewrenFeb 8, 2021
  17. Junio C HamanoFeb 7, 2021
  18. Elijah NewrenFeb 7, 2021
  19. 0/4 Optimization batch 7: use file basenames to guide rename detectionElijah Newren via GitGitGadget, Feb 9, 2021
  20. 2/4 diffcore-rename: complete find_basename_matches()Elijah Newren via GitGitGadget, Feb 9, 2021
  21. Derrick StoleeFeb 9, 2021
  22. Elijah NewrenFeb 9, 2021
  23. Derrick StoleeFeb 9, 2021
  24. 3/4 diffcore-rename: guide inexact rename detection based on basenamesElijah Newren via GitGitGadget, Feb 9, 2021
  25. Derrick StoleeFeb 9, 2021
  26. Elijah NewrenFeb 9, 2021
  27. Junio C HamanoFeb 9, 2021
  28. 1/4 diffcore-rename: compute basenames of all source and dest candidatesElijah Newren via GitGitGadget, Feb 9, 2021
  29. Derrick StoleeFeb 9, 2021
  30. Elijah NewrenFeb 9, 2021
  31. Derrick StoleeFeb 9, 2021
  32. Elijah NewrenFeb 9, 2021
  33. 4/4 gitdiffcore doc: mention new preliminary step for rename detectionElijah Newren via GitGitGadget, Feb 9, 2021
  34. Derrick StoleeFeb 9, 2021
  35. Junio C HamanoFeb 9, 2021
  36. Elijah NewrenFeb 9, 2021
  37. 0/5 Optimization batch 7: use file basenames to guide rename detectionElijah Newren via GitGitGadget, Feb 10, 2021
  38. 1/5 t4001: add a test comparing basename similarity and content similarityElijah Newren via GitGitGadget, Feb 10, 2021
  39. Junio C HamanoFeb 13, 2021
  40. Elijah NewrenFeb 13, 2021
  41. Junio C HamanoFeb 13, 2021
  42. Elijah NewrenFeb 14, 2021
  43. Junio C HamanoFeb 14, 2021
  44. Elijah NewrenFeb 14, 2021
  45. 2/5 diffcore-rename: compute basenames of all source and dest candidatesElijah Newren via GitGitGadget, Feb 10, 2021
  46. Junio C HamanoFeb 13, 2021
  47. 4/5 diffcore-rename: guide inexact rename detection based on basenamesElijah Newren via GitGitGadget, Feb 10, 2021
  48. Junio C HamanoFeb 13, 2021
  49. 3/5 diffcore-rename: complete find_basename_matches()Elijah Newren via GitGitGadget, Feb 10, 2021
  50. Junio C HamanoFeb 13, 2021
  51. Elijah NewrenFeb 13, 2021
  52. Junio C HamanoFeb 13, 2021
  53. Elijah NewrenFeb 14, 2021
  54. 5/5 gitdiffcore doc: mention new preliminary step for rename detectionElijah Newren via GitGitGadget, Feb 10, 2021
  55. Junio C HamanoFeb 10, 2021
  56. Elijah NewrenFeb 10, 2021
  57. 0/6 Optimization batch 7: use file basenames to guide rename detectionElijah Newren via GitGitGadget, Feb 11, 2021
  58. 1/6 t4001: add a test comparing basename similarity and content similarityElijah Newren via GitGitGadget, Feb 11, 2021
  59. 2/6 diffcore-rename: compute basenames of all source and dest candidatesElijah Newren via GitGitGadget, Feb 11, 2021
  60. 3/6 diffcore-rename: complete find_basename_matches()Elijah Newren via GitGitGadget, Feb 11, 2021
  61. 5/6 gitdiffcore doc: mention new preliminary step for rename detectionElijah Newren via GitGitGadget, Feb 11, 2021
  62. 4/6 diffcore-rename: guide inexact rename detection based on basenamesElijah Newren via GitGitGadget, Feb 11, 2021
  63. 6/6 merge-ort: call diffcore_rename() directlyElijah Newren via GitGitGadget, Feb 11, 2021
  64. Junio C HamanoFeb 13, 2021
  65. 0/6 Optimization batch 7: use file basenames to guide rename detectionElijah Newren via GitGitGadget, Feb 14, 2021
  66. 3/6 diffcore-rename: complete find_basename_matches()Elijah Newren via GitGitGadget, Feb 14, 2021
  67. 5/6 gitdiffcore doc: mention new preliminary step for rename detectionElijah Newren via GitGitGadget, Feb 14, 2021
  68. 1/6 t4001: add a test comparing basename similarity and content similarityElijah Newren via GitGitGadget, Feb 14, 2021
  69. 2/6 diffcore-rename: compute basenames of source and dest candidatesElijah Newren via GitGitGadget, Feb 14, 2021
  70. 4/6 diffcore-rename: guide inexact rename detection based on basenamesElijah Newren via GitGitGadget, Feb 14, 2021
  71. 6/6 merge-ort: call diffcore_rename() directlyElijah Newren via GitGitGadget, Feb 14, 2021

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.