D5438: rust-cpython: implementing Graph using C parents function

[gracinet (Georges Racinet)]

gracinet added a comment.


  @yuja yes sorry, forgot to adapt to the new signature. That'll be fixed in next version, and in `revlog.c`, the capsule pointer declaration is now at the top.
  
  About protecting parents() with the GIL `Python<'p>`, I'll have to think more about it, but the solution that comes to mind is as you suggest a two-step wrapping: the long lived `Index` would wrap the actual index object and the function pointer retrieved from the capsule, but would not implement the `Graph` trait directly. A shorter term `GILProtectedIndex` would then implement the `Graph` trait by wrapping the `Index` together with the `Python<'p>` marker coming from a call to the Python interface (methods of the `py_class!` of this series), hence forbidding to release the GIL from any code that calls `Graph::parents()`. I think the current usage is safe, though, because our Python interface methods don't release the GIL, but it's true the `Index` isn't inherentrly safe.
  
  A simpler possibility would be to go the other way: release the GIL from the Python interface methods, and reacquire it at each call of `Graph::parents`. I don't know what the overhead would be, especially with Mercurial being currently monothreaded (unless I'm mistaken), meaning that we don't have benefits to reap. Maybe I'll try that one, since it's so simple, and if I measure negligible overhead, I'll go for it.
  
  As a side note, the same problem could arise with the direct-ffi bindings: the safety promise is held by the caller, this time from C code.

REPOSITORY
  rHG Mercurial

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

To: gracinet, #hg-reviewers
Cc: yuja, durin42, kevincox, mercurial-devel