Patchwork D1768: cext: obtain reference to index entry type

login
register
mail settings
Submitter phabricator
Date Dec. 27, 2017, 12:36 a.m.
Message ID <differential-rev-PHID-DREV-ripiizq4zsdlcjehelht-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26454/
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
  We recently introduced a dedicated type for index entries in the
  pure Python implementation.
  
  This commit tells the revlog C code about that type.
  
  We register the type on the parsers module so it is exposed to
  importers of the module.
  
  In addition, we store a reference to the type on revlog index
  instances. In theory, we could grab the reference from the
  module. However, this requires a fair bit of C code. It is easier
  to just re-import the original module and get a ref from there.
  
  We hold off incrementing the version of the "parsers" extension
  because nothing in core relies on the new API yet.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/cext/revlog.c

CHANGE DETAILS




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


  > We hold off incrementing the version of the "parsers" extension
  >  because nothing in core relies on the new API yet.
  
  It's really minor nit, but we have to increment the version since new
  compiled module doesn't work with old `pure.parsers` module, which
  provides no IndexV1Entry type.

INLINE COMMENTS

> revlog.c:2117
> +
> +	PyModule_AddObject(mod, "IndexV1Entry", index_entry_type);
>  }

Can we be sure that this `IndexV1Entry` is identical to `self->entrytype`
after reloading Python modules?

REPOSITORY
  rHG Mercurial

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

To: indygreg, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Jan. 6, 2018, 7:11 a.m.
indygreg added inline comments.

INLINE COMMENTS

> yuja wrote in revlog.c:2117
> Can we be sure that this `IndexV1Entry` is identical to `self->entrytype`
> after reloading Python modules?

In theory, there could be a mismatch.

If this module is loaded, its `IndexV1Entry` will refer to whatever `mercurial.pure.parsers.IndexV1Entry` is. If `mercurial.pure.parsers` is then reloaded and its `IndexV1Entry` changes and a revlog index type is created, its `IndexV1Entry` will be the new one from the reloaded `mercurial.pure.parsers`.

Since I plan to replace the Python-implemented type with a C backed type, I'm inclined to not care about this.

REPOSITORY
  rHG Mercurial

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

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
@@ -28,6 +28,20 @@ 
 #define PyInt_AsLong PyLong_AsLong
 #endif
 
+PyObject *get_index_entry_type(void) {
+	PyObject *mod, *obj;
+
+	mod = PyImport_ImportModule("mercurial.pure.parsers");
+	if (!mod) {
+		return NULL;
+	}
+
+	obj = PyObject_GetAttrString(mod, "IndexV1Entry");
+	Py_DECREF(mod);
+
+	return obj;
+}
+
 /*
  * A base-16 trie for fast node->rev mapping.
  *
@@ -54,6 +68,7 @@ 
 typedef struct {
 	PyObject_HEAD
 	/* Type-specific fields go here. */
+	PyObject *entrytype;   /* mercurial.pure.parsers.IndexV1Entry type */
 	PyObject *nullentry;   /* Represents an empty/unknown index value */
 	PyObject *data;        /* raw bytes of index */
 	Py_buffer buf;         /* buffer of data */
@@ -1861,6 +1876,7 @@ 
 	PyObject *data_obj, *inlined_obj;
 	Py_ssize_t size;
 
+	self->entrytype = NULL;
 	self->nullentry = NULL;
 	self->raw_length = 0;
 	self->added = NULL;
@@ -1873,6 +1889,11 @@ 
 	self->nt = NULL;
 	self->offsets = NULL;
 
+	self->entrytype = get_index_entry_type();
+	if (!self->entrytype) {
+		return -1;
+	}
+
 	/* Initialize before argument-checking to avoid index_dealloc() crash. */
 	self->nullentry = Py_BuildValue("iiiiiiis#", 0, 0, 0,
 					-1, -1, -1, -1, nullid, 20);
@@ -1937,6 +1958,7 @@ 
 	}
 	Py_XDECREF(self->data);
 	Py_XDECREF(self->added);
+	Py_XDECREF(self->entrytype);
 	Py_XDECREF(self->nullentry);
 	PyObject_Del(self);
 }
@@ -2079,9 +2101,18 @@ 
 
 void revlog_module_init(PyObject *mod)
 {
+	PyObject *index_entry_type;
+
 	indexType.tp_new = PyType_GenericNew;
 	if (PyType_Ready(&indexType) < 0)
 		return;
 	Py_INCREF(&indexType);
 	PyModule_AddObject(mod, "index", (PyObject *)&indexType);
+
+	index_entry_type = get_index_entry_type();
+	if (!index_entry_type) {
+		return;
+	}
+
+	PyModule_AddObject(mod, "IndexV1Entry", index_entry_type);
 }