Patchwork [1,of,7,V3] sparse-revlog: add a `index_get_start` function in C

login
register
mail settings
Submitter Boris Feld
Date Nov. 19, 2018, 9:42 a.m.
Message ID <e1ccc1643335b82f6c0b.1542620533@localhost.localdomain>
Download mbox | patch
Permalink /patch/36641/
State Superseded
Headers show

Comments

Boris Feld - Nov. 19, 2018, 9:42 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1541785348 -3600
#      Fri Nov 09 18:42:28 2018 +0100
# Node ID e1ccc1643335b82f6c0bc94aed969590e15460b1
# Parent  dba590f27c7abacbd7e9b27f3e06822bb0b339cb
# EXP-Topic sparse-perf
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r e1ccc1643335
sparse-revlog: add a `index_get_start` function in C

We are about to implement a native version of `slicechunktodensity`. For
clarity, we introduce the helper functions first. This new function provides
an efficient way to retrieve some of the information needed by
`slicechunktodensity`.
Yuya Nishihara - Nov. 19, 2018, 12:47 p.m.
On Mon, 19 Nov 2018 10:42:13 +0100, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1541785348 -3600
> #      Fri Nov 09 18:42:28 2018 +0100
> # Node ID e1ccc1643335b82f6c0bc94aed969590e15460b1
> # Parent  dba590f27c7abacbd7e9b27f3e06822bb0b339cb
> # EXP-Topic sparse-perf
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r e1ccc1643335
> sparse-revlog: add a `index_get_start` function in C

Just quickly scanned the series.

> --- a/mercurial/cext/revlog.c
> +++ b/mercurial/cext/revlog.c
> @@ -185,6 +185,30 @@ static inline int index_get_parents(inde
>  	return 0;
>  }
>  
> +static inline bool index_get_start(indexObject *self, Py_ssize_t rev,
> +                                   long *result)

Nit: I prefer 0/-1 return value since that's the CPython convention.

> +	long offset;
> +	if (rev >= self->length) {
> +		PyObject *tuple =
> +		    PyList_GET_ITEM(self->added, rev - self->length);
> +		if (!pylong_to_long(PyTuple_GET_ITEM(tuple, 0), &offset)) {
> +			return 0;
> +		};

Perhaps, PyInt_AsSsize_t() is safer since the offset is at most 48bit length,
but sizeof(long) is 32bit on amd64 Windows. Please add the Py3 macro as well.

Sorry, I didn't notice it last time.

> +	} else {
> +		const char *data = index_deref(self, rev);
> +		offset = getbe32(data + 4);
> +		if (rev == 0) /* mask out version number for the first entry */
> +			offset &= 0xFFFF;
> +		else {
> +			uint32_t offset_high = getbe32(data);
> +			offset |= ((uint64_t)offset_high) << 32;
> +		}
> +	}
> +	*result = offset >> 16;
> +	return 1;

Nit: If you prefer a boolean return value, use true/false.

Patch

diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -185,6 +185,30 @@  static inline int index_get_parents(inde
 	return 0;
 }
 
+static inline bool index_get_start(indexObject *self, Py_ssize_t rev,
+                                   long *result)
+{
+	long offset;
+	if (rev >= self->length) {
+		PyObject *tuple =
+		    PyList_GET_ITEM(self->added, rev - self->length);
+		if (!pylong_to_long(PyTuple_GET_ITEM(tuple, 0), &offset)) {
+			return 0;
+		};
+	} else {
+		const char *data = index_deref(self, rev);
+		offset = getbe32(data + 4);
+		if (rev == 0) /* mask out version number for the first entry */
+			offset &= 0xFFFF;
+		else {
+			uint32_t offset_high = getbe32(data);
+			offset |= ((uint64_t)offset_high) << 32;
+		}
+	}
+	*result = offset >> 16;
+	return 1;
+}
+
 /*
  * RevlogNG format (all in big endian, data may be inlined):
  *    6 bytes: offset