{"thread":{"id":"64868","subject":"[PATCH 0/2] xdiff: Remove unneeded members from xrecord_t and xdlclass_t","startedAt":"2026-01-26T10:49:25Z","lastAt":"2026-02-10T20:39:31Z","messageCount":5,"participants":["Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"534656","messageId":"cover.1769424529.git.phillip.wood@dunelm.org.uk","threadId":"64868","inReplyTo":null,"subject":"[PATCH 0/2] xdiff: Remove unneeded members from xrecord_t and xdlclass_t","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-26T10:48:50Z","receivedAt":"2026-01-26T10:49:25Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nThis series has a couple of cleanups on top of 'en/xdiff-cleanup-2'\nthat reduce the sizes of the xrecord_t and xdlclass_t. Unfortunately\nthey conflict with 'en/xdiff-cleanup-3' in seen, in particular with\ndb8a50ca6b9 (xdiff: don't waste time guessing the number of lines,\n2026-01-02). I'm not particularly convinced that moving the call to\nxdl_classify_record() out of xdl_prepare_ctx() in that commit is\na good idea, but if we decide that we do want to stop classifying\nlines in xdl_prepare_ctx() we can start passing the hashes out in a\nseparate array rather than wasting space in xrecord_t.\n\nBase-Commit: 1faf5b085a171f9ba9a6d7a446e0de16acccb1dc\nPublished-As: https://github.com/phillipwood/git/releases/tag/pw%2Fxdiff-cleanup-xrecord_t-and-xdlclass_t%2Fv1\nView-Changes-At: https://github.com/phillipwood/git/compare/1faf5b085...0d251dfba\nFetch-It-Via: git fetch https://github.com/phillipwood/git pw/xdiff-cleanup-xrecord_t-and-xdlclass_t/v1\n\n\nPhillip Wood (2):\n  xdiff: remove \"line_hash\" field from xrecord_t\n  xdiff: remove unused data from xdlclass_t\n\n xdiff/xprepare.c | 20 ++++++++++++--------\n xdiff/xtypes.h   |  1 -\n 2 files changed, 12 insertions(+), 9 deletions(-)\n\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"534657","messageId":"24a662ac0a939d284cb2370509327a2a81535247.1769424529.git.phillip.wood@dunelm.org.uk","threadId":"64868","inReplyTo":"cover.1769424529.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 1/2] xdiff: remove \"line_hash\" field from xrecord_t","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-26T10:48:51Z","receivedAt":"2026-01-26T10:49:25Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nPrior to commit 6a26019c81 (xdiff: split xrecord_t.ha into line_hash\nand minimal_perfect_hash, 2025-11-18) the \"ha\" field of xrecord_t\ninitially held the \"line_hash\" value and once the line had been\ninterned that field was updated to hold the \"minimal_perfect_hash\". The\n\"line_hash\" is only used to intern the line so there is no point in\nstoring it after all the input lines have been interned.\n\nRemoving the \"line_hash\" field from xrecord_t and storing it in\nxdlclass_t where it is actually used makes it clearer that it is a\ntemporary value and it should not be used once we're calculated the\n\"minimal_perfect_hash\". This also reduces the size of xrecord_t by 25%\non 64-bit platforms and 40% on 32-bit platforms. While the struct is\nsmall we create one instance per input line so any saving is welcome.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n xdiff/xprepare.c | 12 +++++++-----\n xdiff/xtypes.h   |  1 -\n 2 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/xdiff/xprepare.c b/xdiff/xprepare.c\nindex 34c82e4f8e1..08e5d3f4dfa 100644\n--- a/xdiff/xprepare.c\n+++ b/xdiff/xprepare.c\n@@ -34,6 +34,7 @@\n #define INVESTIGATE 2\n \n typedef struct s_xdlclass {\n+\tuint64_t line_hash;\n \tstruct s_xdlclass *next;\n \txrecord_t rec;\n \tlong idx;\n@@ -92,13 +93,14 @@ static void xdl_free_classifier(xdlclassifier_t *cf) {\n }\n \n \n-static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t *rec) {\n+static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t *rec,\n+\t\t\t       uint64_t line_hash) {\n \tsize_t hi;\n \txdlclass_t *rcrec;\n \n-\thi = XDL_HASHLONG(rec->line_hash, cf->hbits);\n+\thi = XDL_HASHLONG(line_hash, cf->hbits);\n \tfor (rcrec = cf->rchash[hi]; rcrec; rcrec = rcrec->next)\n-\t\tif (rcrec->rec.line_hash == rec->line_hash &&\n+\t\tif (rcrec->line_hash == line_hash &&\n \t\t\t\txdl_recmatch((const char *)rcrec->rec.ptr, (long)rcrec->rec.size,\n \t\t\t\t\t(const char *)rec->ptr, (long)rec->size, cf->flags))\n \t\t\tbreak;\n@@ -112,6 +114,7 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t\n \t\tif (XDL_ALLOC_GROW(cf->rcrecs, cf->count, cf->alloc))\n \t\t\t\treturn -1;\n \t\tcf->rcrecs[rcrec->idx] = rcrec;\n+\t\trcrec->line_hash = line_hash;\n \t\trcrec->rec = *rec;\n \t\trcrec->len1 = rcrec->len2 = 0;\n \t\trcrec->next = cf->rchash[hi];\n@@ -158,8 +161,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_\n \t\t\tcrec = &xdf->recs[xdf->nrec++];\n \t\t\tcrec->ptr = prev;\n \t\t\tcrec->size = cur - prev;\n-\t\t\tcrec->line_hash = hav;\n-\t\t\tif (xdl_classify_record(pass, cf, crec) < 0)\n+\t\t\tif (xdl_classify_record(pass, cf, crec, hav) < 0)\n \t\t\t\tgoto abort;\n \t\t}\n \t}\ndiff --git a/xdiff/xtypes.h b/xdiff/xtypes.h\nindex 979586f20a6..50aee779be3 100644\n--- a/xdiff/xtypes.h\n+++ b/xdiff/xtypes.h\n@@ -41,7 +41,6 @@ typedef struct s_chastore {\n typedef struct s_xrecord {\n \tuint8_t const *ptr;\n \tsize_t size;\n-\tuint64_t line_hash;\n \tsize_t minimal_perfect_hash;\n } xrecord_t;\n \n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"534658","messageId":"0d251dfba505fe401d46bea912ec107eed0c6ac5.1769424529.git.phillip.wood@dunelm.org.uk","threadId":"64868","inReplyTo":"cover.1769424529.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 2/2] xdiff: remove unused data from xdlclass_t","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-26T10:48:52Z","receivedAt":"2026-01-26T10:49:27Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nPrior to commit 6d507bd41a (xdiff: delete fields ha, line, size\nin xdlclass_t in favor of an xrecord_t, 2025-09-26) xdlclass_t\ncarried a copy of all the fields in xrecord_t. That commit embedded\nxrecord_t in xdlclass_t to make it easier to change the types of\nthe fields in xrecord_t. However commit 6a26019c81 (xdiff: split\nxrecord_t.ha into line_hash and minimal_perfect_hash, 2025-11-18)\nadded the \"minimal_perfect_hash\" field to xrecord_t which is not\nused by xdlclass_t. To avoid wasting space stop copying the whole\nof xrecord_t and just copy the pointer and length that we need to\nintern the line. Together with the previous commit this effectively\nreverts 6d507bd41a.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n xdiff/xprepare.c | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/xdiff/xprepare.c b/xdiff/xprepare.c\nindex 08e5d3f4dfa..cd4fc405eb1 100644\n--- a/xdiff/xprepare.c\n+++ b/xdiff/xprepare.c\n@@ -36,7 +36,8 @@\n typedef struct s_xdlclass {\n \tuint64_t line_hash;\n \tstruct s_xdlclass *next;\n-\txrecord_t rec;\n+\tconst uint8_t *ptr;\n+\tsize_t size;\n \tlong idx;\n \tlong len1, len2;\n } xdlclass_t;\n@@ -101,7 +102,7 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t\n \thi = XDL_HASHLONG(line_hash, cf->hbits);\n \tfor (rcrec = cf->rchash[hi]; rcrec; rcrec = rcrec->next)\n \t\tif (rcrec->line_hash == line_hash &&\n-\t\t\t\txdl_recmatch((const char *)rcrec->rec.ptr, (long)rcrec->rec.size,\n+\t\t\t\txdl_recmatch((const char *)rcrec->ptr, (long)rcrec->size,\n \t\t\t\t\t(const char *)rec->ptr, (long)rec->size, cf->flags))\n \t\t\tbreak;\n \n@@ -115,7 +116,8 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t\n \t\t\t\treturn -1;\n \t\tcf->rcrecs[rcrec->idx] = rcrec;\n \t\trcrec->line_hash = line_hash;\n-\t\trcrec->rec = *rec;\n+\t\trcrec->ptr = rec->ptr;\n+\t\trcrec->size = rec->size;\n \t\trcrec->len1 = rcrec->len2 = 0;\n \t\trcrec->next = cf->rchash[hi];\n \t\tcf->rchash[hi] = rcrec;\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"534679","messageId":"xmqqtsw8i8fa.fsf@gitster.g","threadId":"64868","inReplyTo":"cover.1769424529.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 0/2] xdiff: Remove unneeded members from xrecord_t and xdlclass_t","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-26T17:35:21Z","receivedAt":"2026-01-26T17:35:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>\n> This series has a couple of cleanups on top of 'en/xdiff-cleanup-2'\n> that reduce the sizes of the xrecord_t and xdlclass_t. Unfortunately\n> they conflict with 'en/xdiff-cleanup-3' in seen, in particular with\n> db8a50ca6b9 (xdiff: don't waste time guessing the number of lines,\n> 2026-01-02). I'm not particularly convinced that moving the call to\n> xdl_classify_record() out of xdl_prepare_ctx() in that commit is\n> a good idea, but if we decide that we do want to stop classifying\n> lines in xdl_prepare_ctx() we can start passing the hashes out in a\n> separate array rather than wasting space in xrecord_t.\n\nBoth patches look well reasoned and sensible.\n\nIt is unfortunate that the en/xdiff-cleanup-3 wants to pull these\nfields in a different direction, but the topic has been dormant for\nquite a while, so let's tentatively kick it out of 'seen' and see\nhow well this one does, until we decide how to consolidate the two\ntopics.  Thanks.\n"},{"id":"535711","messageId":"xmqqikc4xri7.fsf@gitster.g","threadId":"64868","inReplyTo":"xmqqtsw8i8fa.fsf@gitster.g","subject":"Re: [PATCH 0/2] xdiff: Remove unneeded members from xrecord_t and xdlclass_t","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-10T20:39:28Z","receivedAt":"2026-02-10T20:39:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>\n>> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>>\n>> This series has a couple of cleanups on top of 'en/xdiff-cleanup-2'\n>> that reduce the sizes of the xrecord_t and xdlclass_t. Unfortunately\n>> they conflict with 'en/xdiff-cleanup-3' in seen, in particular with\n>> db8a50ca6b9 (xdiff: don't waste time guessing the number of lines,\n>> 2026-01-02). I'm not particularly convinced that moving the call to\n>> xdl_classify_record() out of xdl_prepare_ctx() in that commit is\n>> a good idea, but if we decide that we do want to stop classifying\n>> lines in xdl_prepare_ctx() we can start passing the hashes out in a\n>> separate array rather than wasting space in xrecord_t.\n>\n> Both patches look well reasoned and sensible.\n\nI was hoping that these two patches will get reviewed by somebody\nelse in adddition to mine, but unfortunately nothing happened.  I am\ninclined to merge it down so that the other topic can have a stable\nbase to be rebased.\n\nOpinions?\n\n\n"}]}