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

[PATCH 21/26] diff.c: buffer all output if asked to

From
Stefan Beller <sbeller@google.com>
Date
Jun 20, 2017, 02:48 UTC
Message-ID
<20170620024816.20021-22-sbeller@google.com>
In-Reply-To
<20170620024816.20021-1-sbeller@google.com>

Introduce a new option 'emitted_symbols' in the struct diff_options which controls whether all output is buffered up until all output is available. It is set internally in diff.c when necessary.

We'll have a new struct 'emitted_string' in diff.c which will be used to buffer each line. The emitted_string will duplicate the memory of the line to buffer as that is easiest to reason about for now. In a future patch we may want to decrease the memory usage by not duplicating all output for buffering but rather we may want to store offsets into the file or in case of hunk descriptions such as the similarity score, we could just store the relevant number and reproduce the text later on.

This approach was chosen as a first step because it is quite simple compared to the alternative with less memory footprint.

emit_diff_symbol factors out the emission part and depending on the diff_options->emitted_symbols the emission will be performed directly when calling emit_diff_symbol or after the whole process is done, i.e. by buffering we have add the possibility for a second pass over the whole output before doing the actual output.

In 6440d34 (2012-03-14, diff: tweak a _copy_ of diff_options with word-diff) we introduced a duplicate diff options struct for word emissions as we may have different regex settings in there. When buffering the output, we need to operate on just one buffer, so we have to copy back the emissions of the word buffer into the main buffer.

Unconditionally enable output via buffer in this patch as it yields a great opportunity for testing, i.e. all the diff tests from the test suite pass without having reordering issues (i.e. only parts of the output got buffered, and we forgot to buffer other parts). The test suite passes, which gives confidence that we converted all functions to use emit_string for output.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 diff.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 diff.h |   2 ++
 2 files changed, 109 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index f2f7a4da79..35b5924ff2 100644
--- a/diff.c
+++ b/diff.c
@@ -603,6 +603,47 @@ enum diff_symbol {
 #define DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF (1<<16)
 #define DIFF_SYMBOL_CONTENT_WS_MASK (WSEH_NEW | WSEH_OLD | WSEH_CONTEXT | WS_RULE_MASK)
 
+/*
+ * This struct is used when we need to buffer the output of the diff output.
+ *
+ * NEEDSWORK: Instead of storing a copy of the line, add an offset pointer
+ * into the pre/post image file. This pointer could be a union with the
+ * line pointer. By storing an offset into the file instead of the literal line,
+ * we can decrease the memory footprint for the buffered output. At first we
+ * may want to only have indirection for the content lines, but we could also
+ * enhance the state for emitting prefabricated lines, e.g. the similarity
+ * score line or hunk/file headers would only need to store a number or path
+ * and then the output can be constructed later on depending on state.
+ */
+struct emitted_diff_symbol {
+	const char *line;
+	int len;
+	int flags;
+	enum diff_symbol s;
+};
+#define EMITTED_DIFF_SYMBOL_INIT {NULL}
+
+struct emitted_diff_symbols {
+	struct emitted_diff_symbol *buf;
+	int nr, alloc;
+};
+#define EMITTED_DIFF_SYMBOLS_INIT {NULL, 0, 0}
+
+static void append_emitted_diff_symbol(struct diff_options *o,
+				       struct emitted_diff_symbol *e)
+{
+	struct emitted_diff_symbol *f;
+
+	ALLOC_GROW(o->emitted_symbols->buf,
+		   o->emitted_symbols->nr + 1,
+		   o->emitted_symbols->alloc);
+	f = &o->emitted_symbols->buf[o->emitted_symbols->nr++];
+
+	memcpy(f, e, sizeof(struct emitted_diff_symbol));
+	f->line = e->line ? xmemdupz(e->line, e->len) : NULL;
+}
+
+
 static void emit_line_ws_markup(struct diff_options *o,
 				const char *set, const char *reset,
 				const char *line, int len, char sign,
@@ -629,12 +670,18 @@ static void emit_line_ws_markup(struct diff_options *o,
 	}
 }
 
-static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,
-			     const char *line, int len, unsigned flags)
+static void emit_diff_symbol_from_struct(struct diff_options *o,
+					 struct emitted_diff_symbol *eds)
 {
 	static const char *nneof = " No newline at end of file\n";
 	const char *context, *reset, *set, *meta, *fraginfo;
 	struct strbuf sb = STRBUF_INIT;
+
+	enum diff_symbol s = eds->s;
+	const char *line = eds->line;
+	int len = eds->len;
+	unsigned flags = eds->flags;
+
 	switch (s) {
 	case DIFF_SYMBOL_SEPARATOR:
 		fprintf(o->file, "%s%c",
@@ -790,6 +837,17 @@ static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,
 	strbuf_release(&sb);
 }
 
+static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,
+			     const char *line, int len, unsigned flags)
+{
+	struct emitted_diff_symbol e = {line, len, flags, s};
+
+	if (o->emitted_symbols)
+		append_emitted_diff_symbol(o, &e);
+	else
+		emit_diff_symbol_from_struct(o, &e);
+}
+
 void diff_emit_submodule_del(struct diff_options *o, const char *line)
 {
 	emit_diff_symbol(o, DIFF_SYMBOL_SUBMODULE_DEL, line, strlen(line), 0);
@@ -1384,9 +1442,29 @@ static void diff_words_show(struct diff_words_data *diff_words)
 /* In "color-words" mode, show word-diff of words accumulated in the buffer */
 static void diff_words_flush(struct emit_callback *ecbdata)
 {
+	struct diff_options *wo = ecbdata->diff_words->opt;
+
 	if (ecbdata->diff_words->minus.text.size ||
 	    ecbdata->diff_words->plus.text.size)
 		diff_words_show(ecbdata->diff_words);
+
+	if (wo->emitted_symbols) {
+		struct diff_options *o = ecbdata->opt;
+		struct emitted_diff_symbols *wol = wo->emitted_symbols;
+		int i;
+
+		/*
+		 * NEEDSWORK:
+		 * Instead of appending each, concat all words to a line?
+		 */
+		for (i = 0; i < wol->nr; i++)
+			append_emitted_diff_symbol(o, &wol->buf[i]);
+
+		for (i = 0; i < wol->nr; i++)
+			free((void *)wol->buf[i].line);
+
+		wol->nr = 0;
+	}
 }
 
 static void diff_filespec_load_driver(struct diff_filespec *one)
@@ -1422,6 +1500,11 @@ static void init_diff_words_data(struct emit_callback *ecbdata,
 		xcalloc(1, sizeof(struct diff_words_data));
 	ecbdata->diff_words->type = o->word_diff;
 	ecbdata->diff_words->opt = o;
+
+	if (orig_opts->emitted_symbols)
+		o->emitted_symbols =
+			xcalloc(1, sizeof(struct emitted_diff_symbols));
+
 	if (!o->word_regex)
 		o->word_regex = userdiff_word_regex(one);
 	if (!o->word_regex)
@@ -1456,6 +1539,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
 {
 	if (ecbdata->diff_words) {
 		diff_words_flush(ecbdata);
+		free (ecbdata->diff_words->opt->emitted_symbols);
 		free (ecbdata->diff_words->opt);
 		free (ecbdata->diff_words->minus.text.ptr);
 		free (ecbdata->diff_words->minus.orig);
@@ -5008,12 +5092,33 @@ void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)
 static void diff_flush_patch_all_file_pairs(struct diff_options *o)
 {
 	int i;
+	static struct emitted_diff_symbols esm = EMITTED_DIFF_SYMBOLS_INIT;
 	struct diff_queue_struct *q = &diff_queued_diff;
+	/*
+	 * For testing purposes we want to make sure the diff machinery
+	 * works completely with the buffer. If there is anything emitted
+	 * outside the emit_string, then the order is screwed
+	 * up and the tests will fail.
+	 *
+	 * TODO (later in this series):
+	 * We'll unset this pointer in a later patch.
+	 */
+	o->emitted_symbols = &esm;
+
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
 		if (check_pair_status(p))
 			diff_flush_patch(p, o);
 	}
+
+	if (o->emitted_symbols) {
+		for (i = 0; i < esm.nr; i++)
+			emit_diff_symbol_from_struct(o, &esm.buf[i]);
+
+		for (i = 0; i < esm.nr; i++)
+			free((void *)esm.buf[i].line);
+	}
+	esm.nr = 0;
 }
 
 void diff_flush(struct diff_options *options)
diff --git a/diff.h b/diff.h
index b73f7a1e75..65fc9dbb4b 100644
--- a/diff.h
+++ b/diff.h
@@ -186,6 +186,8 @@ struct diff_options {
 	void *output_prefix_data;
 
 	int diff_path_counter;
+
+	struct emitted_diff_symbols *emitted_symbols;
 };
 
 void diff_emit_submodule_del(struct diff_options *o, const char *line);

-- 
2.12.2.575.gb14f27f917
Previous: Stefan BellerNext: Stefan Beller
Message 82 of 125 in “[PATCHv5 00/17] Diff machine: highlight moved lines.”
  1. Stefan BellerMay 24, 2017
  2. 01/17 diff: readability fixStefan Beller, May 24, 2017
  3. 03/17 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, May 24, 2017
  4. 09/17 submodule.c: convert show_submodule_summary to use emit_line_fmtStefan Beller, May 24, 2017
  5. 04/17 diff: introduce more flexible emit functionStefan Beller, May 24, 2017
  6. Jonathan TanJun 13, 2017
  7. Stefan BellerJun 13, 2017
  8. Jonathan TanJun 13, 2017
  9. 05/17 diff.c: convert fn_out_consume to use emit_lineStefan Beller, May 24, 2017
  10. 02/17 diff: move line ending check into emit_hunk_headerStefan Beller, May 24, 2017
  11. 07/17 diff.c: convert emit_rewrite_diff to use emit_line_*Stefan Beller, May 24, 2017
  12. 15/17 diff.c: emit_line includes whitespace highlightingStefan Beller, May 24, 2017
  13. 11/17 diff.c: convert show_stats to use emit_line_*Stefan Beller, May 24, 2017
  14. 17/17 diff.c: color moved lines differentlyStefan Beller, May 24, 2017
  15. Junio C HamanoMay 25, 2017
  16. Stefan BellerMay 25, 2017
  17. 10/17 diff.c: convert emit_binary_diff_body to use emit_line_*Stefan Beller, May 24, 2017
  18. 08/17 diff.c: convert emit_rewrite_lines to use emit_line_*Stefan Beller, May 24, 2017
  19. 16/17 diff: buffer all output if asked toStefan Beller, May 24, 2017
  20. Junio C HamanoMay 25, 2017
  21. Stefan BellerMay 25, 2017
  22. Junio C HamanoMay 26, 2017
  23. Jonathan TanJun 13, 2017
  24. Stefan BellerJun 14, 2017
  25. 12/17 diff.c: convert word diffing to use emit_line_*Stefan Beller, May 24, 2017
  26. 14/17 diff.c: convert diff_summary to use emit_line_*Stefan Beller, May 24, 2017
  27. 13/17 diff.c: convert diff_flush to use emit_line_*Stefan Beller, May 24, 2017
  28. 06/17 diff.c: convert builtin_diff to use emit_line_*Stefan Beller, May 24, 2017
  29. Junio C HamanoMay 25, 2017
  30. Stefan BellerMay 25, 2017
  31. Junio C HamanoMay 26, 2017
  32. Stefan BellerMay 26, 2017
  33. 0/1 Diff machine: highlight moved lines.Stefan Beller, May 27, 2017
  34. 1/1 diff.c: color moved lines differentlyStefan Beller, May 27, 2017
  35. Philip OakleyMay 27, 2017
  36. Stefan BellerMay 30, 2017
  37. diff.c: color moved lines differentlyStefan Beller, Jun 1, 2017
  38. Jonathan TanJun 13, 2017
  39. Stefan BellerJun 14, 2017
  40. 00/26 reroll of sb/diff-color-movedStefan Beller, Jun 20, 2017
  41. 02/26 diff.c: move line ending check into emit_hunk_headerStefan Beller, Jun 20, 2017
  42. 03/26 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, Jun 20, 2017
  43. 06/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_FRAGINFOStefan Beller, Jun 20, 2017
  44. 07/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_NO_LF_EOFStefan Beller, Jun 20, 2017
  45. 08/26 diff.c: migrate emit_line_checked to use emit_diff_symbolStefan Beller, Jun 20, 2017
  46. Junio C HamanoJun 21, 2017
  47. Stefan BellerJun 22, 2017
  48. Stefan BellerJun 22, 2017
  49. Junio C HamanoJun 23, 2017
  50. 11/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIRStefan Beller, Jun 20, 2017
  51. Jonathan TanJun 20, 2017
  52. Junio C HamanoJun 21, 2017
  53. Stefan BellerJun 22, 2017
  54. 04/26 diff.c: introduce emit_diff_symbolStefan Beller, Jun 20, 2017
  55. Junio C HamanoJun 21, 2017
  56. Stefan BellerJun 21, 2017
  57. Junio C HamanoJun 21, 2017
  58. Junio C HamanoJun 21, 2017
  59. Stefan BellerJun 21, 2017
  60. Junio C HamanoJun 21, 2017
  61. Stefan BellerJun 21, 2017
  62. Brandon WilliamsJun 21, 2017
  63. Junio C HamanoJun 21, 2017
  64. Brandon WilliamsJun 21, 2017
  65. 15/26 submodule.c: migrate diff output to use emit_diff_symbolStefan Beller, Jun 20, 2017
  66. Jonathan TanJun 20, 2017
  67. 19/26 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_STAT_SEPStefan Beller, Jun 20, 2017
  68. 12/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_HEADERStefan Beller, Jun 20, 2017
  69. 17/26 diff.c: convert show_stats to use emit_diff_symbolStefan Beller, Jun 20, 2017
  70. Brandon WilliamsJun 21, 2017
  71. Stefan BellerJun 21, 2017
  72. 18/26 diff.c: convert word diffing to use emit_diff_symbolStefan Beller, Jun 20, 2017
  73. 25/26 diff: document the new --color-moved settingStefan Beller, Jun 20, 2017
  74. 14/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_REWRITE_DIFFStefan Beller, Jun 20, 2017
  75. 26/26 diff.c: have a "machine parseable" move coloringStefan Beller, Jun 20, 2017
  76. Stefan BellerJun 20, 2017
  77. Ævar Arnfjörð BjarmasonJun 23, 2017
  78. Brandon WilliamsJun 21, 2017
  79. Junio C HamanoJun 21, 2017
  80. Stefan BellerJun 21, 2017
  81. 20/26 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARYStefan Beller, Jun 20, 2017
  82. 21/26 diff.c: buffer all output if asked toStefan Beller, Jun 20, 2017
  83. 24/26 diff.c: add dimming to moved line detectionStefan Beller, Jun 20, 2017
  84. Junio C HamanoJun 21, 2017
  85. 16/26 diff.c: convert emit_binary_diff_body to use emit_diff_symbolStefan Beller, Jun 20, 2017
  86. Junio C HamanoJun 21, 2017
  87. 23/26 diff.c: color moved lines differently, plain modeStefan Beller, Jun 20, 2017
  88. 09/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_WORDS{_PORCELAIN}Stefan Beller, Jun 20, 2017
  89. 22/26 diff.c: color moved lines differentlyStefan Beller, Jun 20, 2017
  90. Jonathan TanJun 20, 2017
  91. Stefan BellerJun 20, 2017
  92. 05/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_MARKERStefan Beller, Jun 20, 2017
  93. 10/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_INCOMPLETEStefan Beller, Jun 20, 2017
  94. 13/26 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_BINARY_FILESStefan Beller, Jun 20, 2017
  95. Junio C HamanoJun 21, 2017
  96. Stefan BellerJun 21, 2017
  97. 01/26 diff.c: readability fixStefan Beller, Jun 20, 2017
  98. 00/25 reroll of sb/diff-color-movedStefan Beller, Jun 23, 2017
  99. 02/25 diff.c: move line ending check into emit_hunk_headerStefan Beller, Jun 23, 2017
  100. 04/25 diff.c: introduce emit_diff_symbolStefan Beller, Jun 23, 2017
  101. Junio C HamanoJun 23, 2017
  102. Stefan BellerJun 23, 2017
  103. 06/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_FRAGINFOStefan Beller, Jun 23, 2017
  104. 03/25 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, Jun 23, 2017
  105. 05/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_MARKERStefan Beller, Jun 23, 2017
  106. 09/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_WORDS[_PORCELAIN]Stefan Beller, Jun 23, 2017
  107. 07/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_NO_LF_EOFStefan Beller, Jun 23, 2017
  108. 13/25 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_BINARY_FILESStefan Beller, Jun 23, 2017
  109. 12/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_HEADERStefan Beller, Jun 23, 2017
  110. 14/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_REWRITE_DIFFStefan Beller, Jun 23, 2017
  111. 23/25 diff.c: color moved lines differently, plain modeStefan Beller, Jun 23, 2017
  112. 22/25 diff.c: color moved lines differentlyStefan Beller, Jun 23, 2017
  113. 15/25 submodule.c: migrate diff output to use emit_diff_symbolStefan Beller, Jun 23, 2017
  114. 25/25 diff: document the new --color-moved settingStefan Beller, Jun 23, 2017
  115. 11/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIR_{PLUS, MINUS}Stefan Beller, Jun 23, 2017
  116. 24/25 diff.c: add dimming to moved line detectionStefan Beller, Jun 23, 2017
  117. 01/25 diff.c: readability fixStefan Beller, Jun 23, 2017
  118. 10/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_INCOMPLETEStefan Beller, Jun 23, 2017
  119. 21/25 diff.c: buffer all output if asked toStefan Beller, Jun 23, 2017
  120. 08/25 diff.c: migrate emit_line_checked to use emit_diff_symbolStefan Beller, Jun 23, 2017
  121. 18/25 diff.c: convert word diffing to use emit_diff_symbolStefan Beller, Jun 23, 2017
  122. 17/25 diff.c: convert show_stats to use emit_diff_symbolStefan Beller, Jun 23, 2017
  123. 19/25 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_STAT_SEPStefan Beller, Jun 23, 2017
  124. 20/25 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARYStefan Beller, Jun 23, 2017
  125. 16/25 diff.c: convert emit_binary_diff_body to use emit_diff_symbolStefan Beller, Jun 23, 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.