Patchwork D1767: cext: make nullentry a member of index types

login
register
mail settings
Submitter phabricator
Date Dec. 27, 2017, 12:36 a.m.
Message ID <differential-rev-PHID-DREV-4l2jeiwjnyuv3fl5om47-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26453/
State New
Headers show

Comments

phabricator - Dec. 27, 2017, 12:36 a.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  An upcoming commit will change the type of index entries from tuple
  to something else. This type may not be initialized/available at
  module init time.
  
  We prepare for this future change by making null entries a per-instance
  variable on the index type.
  
  In theory, this does add some overhead (a new 8-tuple per index
  instance). However, the overhead should be negligible compared to
  the run-time overhead of creating an index entry for each revision
  in the index.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1767

AFFECTED FILES
  mercurial/cext/revlog.c

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - Jan. 5, 2018, 3:53 a.m.
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> revlog.c:1876
>  
> +	/* Initialize before argument-checking to avoid index_dealloc() crash. */
> +	self->nullentry = Py_BuildValue("iiiiiiis#", 0, 0, 0,

This comment shouldn't be moved. We need to NULLify all attributes, but don't
have to create PyObject.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1767

To: indygreg, #hg-reviewers, yuja
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -54,6 +54,7 @@ 
 typedef struct {
 	PyObject_HEAD
 	/* Type-specific fields go here. */
+	PyObject *nullentry;   /* Represents an empty/unknown index value */
 	PyObject *data;        /* raw bytes of index */
 	Py_buffer buf;         /* buffer of data */
 	PyObject **cache;      /* cached tuples */
@@ -81,7 +82,6 @@ 
 	return self->length + PyList_GET_SIZE(self->added);
 }
 
-static PyObject *nullentry;
 static const char nullid[20];
 
 static Py_ssize_t inline_scan(indexObject *self, const char **offsets);
@@ -167,8 +167,8 @@ 
 	}
 
 	if (pos == length - 1) {
-		Py_INCREF(nullentry);
-		return nullentry;
+		Py_INCREF(self->nullentry);
+		return self->nullentry;
 	}
 
 	if (pos >= self->length - 1) {
@@ -1861,7 +1861,7 @@ 
 	PyObject *data_obj, *inlined_obj;
 	Py_ssize_t size;
 
-	/* Initialize before argument-checking to avoid index_dealloc() crash. */
+	self->nullentry = NULL;
 	self->raw_length = 0;
 	self->added = NULL;
 	self->cache = NULL;
@@ -1873,6 +1873,13 @@ 
 	self->nt = NULL;
 	self->offsets = NULL;
 
+	/* Initialize before argument-checking to avoid index_dealloc() crash. */
+	self->nullentry = Py_BuildValue("iiiiiiis#", 0, 0, 0,
+					-1, -1, -1, -1, nullid, 20);
+	if (!self->nullentry) {
+		return -1;
+	}
+
 	if (!PyArg_ParseTuple(args, "OO", &data_obj, &inlined_obj))
 		return -1;
 	if (!PyObject_CheckBuffer(data_obj)) {
@@ -1930,6 +1937,7 @@ 
 	}
 	Py_XDECREF(self->data);
 	Py_XDECREF(self->added);
+	Py_XDECREF(self->nullentry);
 	PyObject_Del(self);
 }
 
@@ -2076,9 +2084,4 @@ 
 		return;
 	Py_INCREF(&indexType);
 	PyModule_AddObject(mod, "index", (PyObject *)&indexType);
-
-	nullentry = Py_BuildValue("iiiiiiis#", 0, 0, 0,
-				  -1, -1, -1, -1, nullid, 20);
-	if (nullentry)
-		PyObject_GC_UnTrack(nullentry);
 }