This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12137

revlog: split revlog v1 and revlog v2 handling
ClosedPublic

Authored by pacien on Feb 7 2022, 6:43 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG92b5a2c4d637: revlog: split revlog v1 and revlog v2 handling
Summary

Explicitly splitting their fields packing and unpacking makes it easier to
extend the existing C implemenation to handle the new changelog format, whose
fields and offsets are not simply a superset of the revlog.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pacien created this revision.Feb 7 2022, 6:43 AM
joerg.sonnenberger added a subscriber: joerg.sonnenberger.Feb 7 2022, 6:59 AM

I'm not a fan of this change, it seems a step in the wrong direction.

Alphare added a subscriber: Alphare.Feb 7 2022, 8:28 AM
In D12137#186120, @joerg.sonnenberger wrote:

I'm not a fan of this change, it seems a step in the wrong direction.

Care to elaborate? I'll review both patches soon, so I want to make sure I understand your point beforehand

joerg.sonnenberger added a comment.Feb 7 2022, 9:26 AM

Most of v1 and v2 is shared and duplicating all of that just adds complications for no gain.

Alphare added a comment.Feb 7 2022, 9:51 AM
In D12137#186147, @joerg.sonnenberger wrote:

Most of v1 and v2 is shared and duplicating all of that just adds complications for no gain.

It is now, but this will change shortly from what I understand. Maybe @pacien can elaborate?

Splitting the main logic seems like a big gain to be, although I really wish we could forgo this very manual type of coding, but we have to have a C variant for those core things.

pacien added a comment.Feb 7 2022, 1:35 PM

This patch contains some preparatory work for the C implementation of
changelogv2 which changes the record structure, not simply extending
the existing formats.

The different format versions being independently defined, I think it
makes more sense to handle them independently explicitly, avoiding
tangling versions together to factor what they happen to have in common.

The benefit here is added clarity by splitting the legacy, current and
future (experimental) formats, limiting the risk of breakage of the ones
already frozen.

pacien added a comment.Feb 7 2022, 1:41 PM

This patch contains some preparatory work for the C implementation of
changelogv2 which changes the record structure, not simply extending
the existing formats.

The different format versions being independently defined, I think it
makes more sense to handle them independently explicitly, avoiding
tangling versions together to factor what they happen to have in common.

The benefit here is added clarity by splitting the legacy, current and
future (experimental) formats, limiting the risk of breakage of the ones
already frozen.

Alphare accepted this revision.Feb 8 2022, 8:43 AM

Splitting both implementations is a good idea IMO since they'll be quite different pretty soon. The change looks good by itself.

This revision is now accepted and ready to land.Feb 8 2022, 8:43 AM
pacien added a commit: rHG92b5a2c4d637: revlog: split revlog v1 and revlog v2 handling.Feb 8 2022, 10:44 AM
Closed by commit rHG92b5a2c4d637: revlog: split revlog v1 and revlog v2 handling (authored by pacien). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/cext/revlog.c (270 lines)

Diff 32092

mercurial/cext/revlog.c

static const long v1_entry_size = 64; static const long v1_entry_size = 64;
/* A Revlogv2 index entry is 96 bytes long. */ /* A Revlogv2 index entry is 96 bytes long. */
static const long v2_entry_size = 96; static const long v2_entry_size = 96;
static const long format_v1 = 1; /* Internal only, could be any number */ static const long format_v1 = 1; /* Internal only, could be any number */
static const long format_v2 = 2; /* Internal only, could be any number */ static const long format_v2 = 2; /* Internal only, could be any number */
static const long entry_offset_high = 0; static const long entry_v1_offset_high = 0;
static const long entry_offset_offset_flags = 4; static const long entry_v1_offset_offset_flags = 4;
static const long entry_offset_comp_len = 8; static const long entry_v1_offset_comp_len = 8;
static const long entry_offset_uncomp_len = 12; static const long entry_v1_offset_uncomp_len = 12;
static const long entry_offset_base_rev = 16; static const long entry_v1_offset_base_rev = 16;
static const long entry_offset_link_rev = 20; static const long entry_v1_offset_link_rev = 20;
static const long entry_offset_parent_1 = 24; static const long entry_v1_offset_parent_1 = 24;
static const long entry_offset_parent_2 = 28; static const long entry_v1_offset_parent_2 = 28;
static const long entry_offset_node_id = 32; static const long entry_v1_offset_node_id = 32;
static const long entry_offset_sidedata_offset = 64;
static const long entry_offset_sidedata_comp_len = 72; static const long entry_v2_offset_high = 0;
static const long entry_offset_all_comp_mode = 76; static const long entry_v2_offset_offset_flags = 4;
static const long entry_offset_padding_start = 77; static const long entry_v2_offset_comp_len = 8;
static const long entry_v2_offset_uncomp_len = 12;
static const long entry_v2_offset_base_rev = 16;
static const long entry_v2_offset_link_rev = 20;
static const long entry_v2_offset_parent_1 = 24;
static const long entry_v2_offset_parent_2 = 28;
static const long entry_v2_offset_node_id = 32;
static const long entry_v2_offset_sidedata_offset = 64;
static const long entry_v2_offset_sidedata_comp_len = 72;
static const long entry_v2_offset_all_comp_mode = 76;
static const long entry_v2_offset_padding_start = 77;
static const char comp_mode_inline = 2; static const char comp_mode_inline = 2;
static const char rank_unknown = -1; static const char rank_unknown = -1;
static void raise_revlog_error(void) static void raise_revlog_error(void)
{ {
PyObject *mod = NULL, *dict = NULL, *errclass = NULL; PyObject *mod = NULL, *dict = NULL, *errclass = NULL;
* The specified rev must be valid and must not be nullrev. A returned * The specified rev must be valid and must not be nullrev. A returned
* parent revision may be nullrev, but is guaranteed to be in valid range. * parent revision may be nullrev, but is guaranteed to be in valid range.
*/ */
static inline int index_get_parents(indexObject *self, Py_ssize_t rev, int *ps, static inline int index_get_parents(indexObject *self, Py_ssize_t rev, int *ps,
int maxrev) int maxrev)
{ {
const char *data = index_deref(self, rev); const char *data = index_deref(self, rev);
ps[0] = getbe32(data + entry_offset_parent_1); if (self->format_version == format_v1) {
ps[1] = getbe32(data + entry_offset_parent_2); ps[0] = getbe32(data + entry_v1_offset_parent_1);
ps[1] = getbe32(data + entry_v1_offset_parent_2);
} else if (self->format_version == format_v2) {
ps[0] = getbe32(data + entry_v2_offset_parent_1);
ps[1] = getbe32(data + entry_v2_offset_parent_2);
} else {
raise_revlog_error();
return -1;
}
/* If index file is corrupted, ps[] may point to invalid revisions. So /* If index file is corrupted, ps[] may point to invalid revisions. So
* there is a risk of buffer overflow to trust them unconditionally. */ * there is a risk of buffer overflow to trust them unconditionally. */
if (ps[0] < -1 || ps[0] > maxrev || ps[1] < -1 || ps[1] > maxrev) { if (ps[0] < -1 || ps[0] > maxrev || ps[1] < -1 || ps[1] > maxrev) {
PyErr_SetString(PyExc_ValueError, "parent out of range"); PyErr_SetString(PyExc_ValueError, "parent out of range");
return -1; return -1;
} }
return 0; return 0;
{ {
const char *data; const char *data;
uint64_t offset; uint64_t offset;
if (rev == nullrev) if (rev == nullrev)
return 0; return 0;
data = index_deref(self, rev); data = index_deref(self, rev);
offset = getbe32(data + entry_offset_offset_flags);
if (self->format_version == format_v1) {
offset = getbe32(data + entry_v1_offset_offset_flags);
if (rev == 0) {
/* mask out version number for the first entry */
offset &= 0xFFFF;
} else {
uint32_t offset_high =
getbe32(data + entry_v1_offset_high);
offset |= ((uint64_t)offset_high) << 32;
}
} else if (self->format_version == format_v2) {
offset = getbe32(data + entry_v2_offset_offset_flags);
if (rev == 0) { if (rev == 0) {
/* mask out version number for the first entry */ /* mask out version number for the first entry */
offset &= 0xFFFF; offset &= 0xFFFF;
} else { } else {
uint32_t offset_high = getbe32(data + entry_offset_high); uint32_t offset_high =
getbe32(data + entry_v2_offset_high);
offset |= ((uint64_t)offset_high) << 32; offset |= ((uint64_t)offset_high) << 32;
} }
} else {
raise_revlog_error();
return -1;
}
return (int64_t)(offset >> 16); return (int64_t)(offset >> 16);
} }
static inline int index_get_length(indexObject *self, Py_ssize_t rev) static inline int index_get_length(indexObject *self, Py_ssize_t rev)
{ {
const char *data; const char *data;
int tmp; int tmp;
if (rev == nullrev) if (rev == nullrev)
return 0; return 0;
data = index_deref(self, rev); data = index_deref(self, rev);
tmp = (int)getbe32(data + entry_offset_comp_len); if (self->format_version == format_v1) {
tmp = (int)getbe32(data + entry_v1_offset_comp_len);
} else if (self->format_version == format_v2) {
tmp = (int)getbe32(data + entry_v2_offset_comp_len);
} else {
raise_revlog_error();
return -1;
}
if (tmp < 0) { if (tmp < 0) {
PyErr_Format(PyExc_OverflowError, PyErr_Format(PyExc_OverflowError,
"revlog entry size out of bound (%d)", tmp); "revlog entry size out of bound (%d)", tmp);
return -1; return -1;
} }
return tmp; return tmp;
} }
PyErr_SetString(PyExc_IndexError, "revlog index out of range"); PyErr_SetString(PyExc_IndexError, "revlog index out of range");
return NULL; return NULL;
} }
data = index_deref(self, pos); data = index_deref(self, pos);
if (data == NULL) if (data == NULL)
return NULL; return NULL;
offset_flags = getbe32(data + entry_offset_offset_flags); if (self->format_version == format_v1) {
offset_flags = getbe32(data + entry_v1_offset_offset_flags);
/* /*
* The first entry on-disk needs the version number masked out, * The first entry on-disk needs the version number masked out,
* but this doesn't apply if entries are added to an empty index. * but this doesn't apply if entries are added to an empty
* index.
*/ */
if (self->length && pos == 0) if (self->length && pos == 0)
offset_flags &= 0xFFFF; offset_flags &= 0xFFFF;
else { else {
uint32_t offset_high = getbe32(data + entry_offset_high); uint32_t offset_high =
getbe32(data + entry_v1_offset_high);
offset_flags |= ((uint64_t)offset_high) << 32; offset_flags |= ((uint64_t)offset_high) << 32;
} }
comp_len = getbe32(data + entry_offset_comp_len); comp_len = getbe32(data + entry_v1_offset_comp_len);
uncomp_len = getbe32(data + entry_offset_uncomp_len); uncomp_len = getbe32(data + entry_v1_offset_uncomp_len);
base_rev = getbe32(data + entry_offset_base_rev); base_rev = getbe32(data + entry_v1_offset_base_rev);
link_rev = getbe32(data + entry_offset_link_rev); link_rev = getbe32(data + entry_v1_offset_link_rev);
parent_1 = getbe32(data + entry_offset_parent_1); parent_1 = getbe32(data + entry_v1_offset_parent_1);
parent_2 = getbe32(data + entry_offset_parent_2); parent_2 = getbe32(data + entry_v1_offset_parent_2);
c_node_id = data + entry_offset_node_id; c_node_id = data + entry_v1_offset_node_id;
if (self->format_version == format_v1) {
sidedata_offset = 0; sidedata_offset = 0;
sidedata_comp_len = 0; sidedata_comp_len = 0;
data_comp_mode = comp_mode_inline; data_comp_mode = comp_mode_inline;
sidedata_comp_mode = comp_mode_inline; sidedata_comp_mode = comp_mode_inline;
} else { } else if (self->format_version == format_v2) {
sidedata_offset = getbe64(data + entry_offset_sidedata_offset); offset_flags = getbe32(data + entry_v2_offset_offset_flags);
/*
* The first entry on-disk needs the version number masked out,
* but this doesn't apply if entries are added to an empty
* index.
*/
if (self->length && pos == 0)
offset_flags &= 0xFFFF;
else {
uint32_t offset_high =
getbe32(data + entry_v2_offset_high);
offset_flags |= ((uint64_t)offset_high) << 32;
}
comp_len = getbe32(data + entry_v2_offset_comp_len);
uncomp_len = getbe32(data + entry_v2_offset_uncomp_len);
base_rev = getbe32(data + entry_v2_offset_base_rev);
link_rev = getbe32(data + entry_v2_offset_link_rev);
parent_1 = getbe32(data + entry_v2_offset_parent_1);
parent_2 = getbe32(data + entry_v2_offset_parent_2);
c_node_id = data + entry_v2_offset_node_id;
sidedata_offset =
getbe64(data + entry_v2_offset_sidedata_offset);
sidedata_comp_len = sidedata_comp_len =
getbe32(data + entry_offset_sidedata_comp_len); getbe32(data + entry_v2_offset_sidedata_comp_len);
data_comp_mode = data[entry_offset_all_comp_mode] & 3; data_comp_mode = data[entry_v2_offset_all_comp_mode] & 3;
sidedata_comp_mode = sidedata_comp_mode =
((data[entry_offset_all_comp_mode] >> 2) & 3); ((data[entry_v2_offset_all_comp_mode] >> 2) & 3);
} else {
raise_revlog_error();
return NULL;
} }
return Py_BuildValue(tuple_format, offset_flags, comp_len, uncomp_len, return Py_BuildValue(tuple_format, offset_flags, comp_len, uncomp_len,
base_rev, link_rev, parent_1, parent_2, c_node_id, base_rev, link_rev, parent_1, parent_2, c_node_id,
self->nodelen, sidedata_offset, sidedata_comp_len, self->nodelen, sidedata_offset, sidedata_comp_len,
data_comp_mode, sidedata_comp_mode, rank_unknown); data_comp_mode, sidedata_comp_mode, rank_unknown);
} }
/* /*
/* /*
* Return the hash of node corresponding to the given rev. * Return the hash of node corresponding to the given rev.
*/ */
static const char *index_node(indexObject *self, Py_ssize_t pos) static const char *index_node(indexObject *self, Py_ssize_t pos)
{ {
Py_ssize_t length = index_length(self); Py_ssize_t length = index_length(self);
const char *data; const char *data;
const char *node_id;
if (pos == nullrev) if (pos == nullrev)
return nullid; return nullid;
if (pos >= length) if (pos >= length)
return NULL; return NULL;
data = index_deref(self, pos); data = index_deref(self, pos);
return data ? data + entry_offset_node_id : NULL;
if (self->format_version == format_v1) {
node_id = data + entry_v1_offset_node_id;
} else if (self->format_version == format_v2) {
node_id = data + entry_v2_offset_node_id;
} else {
raise_revlog_error();
return NULL;
}
return data ? node_id : NULL;
} }
/* /*
* Return the hash of the node corresponding to the given rev. The * Return the hash of the node corresponding to the given rev. The
* rev is assumed to be existing. If not, an exception is set. * rev is assumed to be existing. If not, an exception is set.
*/ */
static const char *index_node_existing(indexObject *self, Py_ssize_t pos) static const char *index_node_existing(indexObject *self, Py_ssize_t pos)
{ {
self->added, new_added_length * self->entry_size); self->added, new_added_length * self->entry_size);
if (!new_added) if (!new_added)
return PyErr_NoMemory(); return PyErr_NoMemory();
self->added = new_added; self->added = new_added;
self->added_length = new_added_length; self->added_length = new_added_length;
} }
rev = self->length + self->new_length; rev = self->length + self->new_length;
data = self->added + self->entry_size * self->new_length++; data = self->added + self->entry_size * self->new_length++;
putbe32(offset_flags >> 32, data + entry_offset_high);
putbe32(offset_flags & 0xffffffffU, data + entry_offset_offset_flags); if (self->format_version == format_v1) {
putbe32(comp_len, data + entry_offset_comp_len); putbe32(offset_flags >> 32, data + entry_v1_offset_high);
putbe32(uncomp_len, data + entry_offset_uncomp_len); putbe32(offset_flags & 0xffffffffU,
putbe32(base_rev, data + entry_offset_base_rev); data + entry_v1_offset_offset_flags);
putbe32(link_rev, data + entry_offset_link_rev); putbe32(comp_len, data + entry_v1_offset_comp_len);
putbe32(parent_1, data + entry_offset_parent_1); putbe32(uncomp_len, data + entry_v1_offset_uncomp_len);
putbe32(parent_2, data + entry_offset_parent_2); putbe32(base_rev, data + entry_v1_offset_base_rev);
memcpy(data + entry_offset_node_id, c_node_id, c_node_id_len); putbe32(link_rev, data + entry_v1_offset_link_rev);
putbe32(parent_1, data + entry_v1_offset_parent_1);
putbe32(parent_2, data + entry_v1_offset_parent_2);
memcpy(data + entry_v1_offset_node_id, c_node_id,
c_node_id_len);
/* Padding since SHA-1 is only 20 bytes for now */ /* Padding since SHA-1 is only 20 bytes for now */
memset(data + entry_offset_node_id + c_node_id_len, 0, memset(data + entry_v1_offset_node_id + c_node_id_len, 0,
entry_offset_node_id - c_node_id_len); entry_v1_offset_node_id - c_node_id_len);
if (self->format_version == format_v2) { } else if (self->format_version == format_v2) {
putbe64(sidedata_offset, data + entry_offset_sidedata_offset); putbe32(offset_flags >> 32, data + entry_v2_offset_high);
putbe32(offset_flags & 0xffffffffU,
data + entry_v2_offset_offset_flags);
putbe32(comp_len, data + entry_v2_offset_comp_len);
putbe32(uncomp_len, data + entry_v2_offset_uncomp_len);
putbe32(base_rev, data + entry_v2_offset_base_rev);
putbe32(link_rev, data + entry_v2_offset_link_rev);
putbe32(parent_1, data + entry_v2_offset_parent_1);
putbe32(parent_2, data + entry_v2_offset_parent_2);
memcpy(data + entry_v2_offset_node_id, c_node_id,
c_node_id_len);
/* Padding since SHA-1 is only 20 bytes for now */
memset(data + entry_v2_offset_node_id + c_node_id_len, 0,
entry_v2_offset_node_id - c_node_id_len);
putbe64(sidedata_offset,
data + entry_v2_offset_sidedata_offset);
putbe32(sidedata_comp_len, putbe32(sidedata_comp_len,
data + entry_offset_sidedata_comp_len); data + entry_v2_offset_sidedata_comp_len);
comp_field = data_comp_mode & 3; comp_field = data_comp_mode & 3;
comp_field = comp_field | (sidedata_comp_mode & 3) << 2; comp_field = comp_field | (sidedata_comp_mode & 3) << 2;
data[entry_offset_all_comp_mode] = comp_field; data[entry_v2_offset_all_comp_mode] = comp_field;
/* Padding for 96 bytes alignment */ /* Padding for 96 bytes alignment */
memset(data + entry_offset_padding_start, 0, memset(data + entry_v2_offset_padding_start, 0,
self->entry_size - entry_offset_padding_start); self->entry_size - entry_v2_offset_padding_start);
} else {
raise_revlog_error();
return NULL;
} }
if (self->ntinitialized) if (self->ntinitialized)
nt_insert(&self->nt, c_node_id, rev); nt_insert(&self->nt, c_node_id, rev);
Py_CLEAR(self->headrevs); Py_CLEAR(self->headrevs);
Py_RETURN_NONE; Py_RETURN_NONE;
} }
PyExc_IndexError, PyExc_IndexError,
"cannot rewrite entries outside of this transaction"); "cannot rewrite entries outside of this transaction");
return NULL; return NULL;
} }
/* Find the newly added node, offset from the "already on-disk" length /* Find the newly added node, offset from the "already on-disk" length
*/ */
data = self->added + self->entry_size * (rev - self->length); data = self->added + self->entry_size * (rev - self->length);
putbe64(offset_flags, data + entry_offset_high); putbe64(offset_flags, data + entry_v2_offset_high);
putbe64(sidedata_offset, data + entry_offset_sidedata_offset); putbe64(sidedata_offset, data + entry_v2_offset_sidedata_offset);
putbe32(sidedata_comp_len, data + entry_offset_sidedata_comp_len); putbe32(sidedata_comp_len, data + entry_v2_offset_sidedata_comp_len);
data[entry_offset_all_comp_mode] = data[entry_v2_offset_all_comp_mode] =
(data[entry_offset_all_comp_mode] & ~(3 << 2)) | (data[entry_v2_offset_all_comp_mode] & ~(3 << 2)) |
((comp_mode & 3) << 2); ((comp_mode & 3) << 2);
Py_RETURN_NONE; Py_RETURN_NONE;
} }
static PyObject *index_stats(indexObject *self) static PyObject *index_stats(indexObject *self)
{ {
PyObject *obj = PyDict_New(); PyObject *obj = PyDict_New();
static inline int index_baserev(indexObject *self, int rev) static inline int index_baserev(indexObject *self, int rev)
{ {
const char *data; const char *data;
int result; int result;
data = index_deref(self, rev); data = index_deref(self, rev);
if (data == NULL) if (data == NULL)
return -2; return -2;
result = getbe32(data + entry_offset_base_rev);
if (self->format_version == format_v1) {
result = getbe32(data + entry_v1_offset_base_rev);
} else if (self->format_version == format_v2) {
result = getbe32(data + entry_v2_offset_base_rev);
} else {
raise_revlog_error();
return -1;
}
if (result > rev) { if (result > rev) {
PyErr_Format( PyErr_Format(
PyExc_ValueError, PyExc_ValueError,
"corrupted revlog, revision base above revision: %d, %d", "corrupted revlog, revision base above revision: %d, %d",
rev, result); rev, result);
return -2; return -2;
} }
Py_ssize_t pos = 0; Py_ssize_t pos = 0;
Py_ssize_t end = self->buf.len; Py_ssize_t end = self->buf.len;
long incr = self->entry_size; long incr = self->entry_size;
Py_ssize_t len = 0; Py_ssize_t len = 0;
while (pos + self->entry_size <= end && pos >= 0) { while (pos + self->entry_size <= end && pos >= 0) {
uint32_t comp_len, sidedata_comp_len = 0; uint32_t comp_len, sidedata_comp_len = 0;
/* 3rd element of header is length of compressed inline data */ /* 3rd element of header is length of compressed inline data */
comp_len = getbe32(data + pos + entry_offset_comp_len); if (self->format_version == format_v1) {
if (self->entry_size == v2_entry_size) { comp_len =
getbe32(data + pos + entry_v1_offset_comp_len);
sidedata_comp_len = 0;
} else if (self->format_version == format_v2) {
comp_len =
getbe32(data + pos + entry_v2_offset_comp_len);
sidedata_comp_len = getbe32( sidedata_comp_len = getbe32(
data + pos + entry_offset_sidedata_comp_len); data + pos + entry_v2_offset_sidedata_comp_len);
} else {
raise_revlog_error();
return -1;
} }
incr = self->entry_size + comp_len + sidedata_comp_len; incr = self->entry_size + comp_len + sidedata_comp_len;
if (offsets) if (offsets)
offsets[len] = data + pos; offsets[len] = data + pos;
len++; len++;
pos += incr; pos += incr;
} }