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

[yuja (Yuya Nishihara)]

yuja added a comment.


  >   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.
  
  Actually I like this idea since it will be a step forward to wrap the Index
  object properly as a PyObject.
  
  >   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.
  
  Yes. The current code is safe unless we do invoke it with e.g.
  `py.allow_threads()`, and I'm not so afraid of leaving the issue as TODO.

REPOSITORY
  rHG Mercurial

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

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