WeakPersistent Redesign

[agnat]

This is a follow-up on #210.

@kkoopa mentioned in a comment that the weak persistent callback stuff could use some love:

If you're still looking for a challenge, the weak callback stuff is a bit uglier than might be necessary, should it not be possible to replace the dispatcher with a template function around a functor (would not really want a function pointer, as that is no improvement on the current situation), with the same syntax as currently used?

Also, NAN_WEAK_CALLBACK does not fully work as NAN_METHOD and friends, you cannot use it to do a forward declaration and there is some fudging with class members due to the built-in static keyword which is needed sometimes, but not always, and cannot be set from user code due to it being a template declaration, meaning that static NAN_WEAK_CALLBACK(foo) puts the static keyword in the wrong place. This is something that still has not been fully resolved.

Let's discuss it.

I'm still in the process of wrapping my head around the current stuff but one thing seems obvious: It is not very flexible. Compared to NanNew<T>(...) it is totally rigid. I think the right thing to do is to get rid of NAN_WEAK_CALLBACK all together. Let users define the callbacks more freely (free function, static member function, functor, member function, what ever) and then deal with it in NanMakeWeakPersistent(...). I'd also like to do away with that ugly argument.

Pseudo:

void f0(v8::Handle<v8::Value> trash) {}
void f1(v8::Handle<v8::Object> trash, int * param) {}
struct foo {
  static void bar(v8::Handle<v8::String> trash, foo * param) {}
};

NanMakeWeakPersistent(obj, f0); 
NanMakeWeakPersistent(obj, f1, new int());
NanMakeWeakPersistent(obj, foo::bar, new foo());

I might be a bit optimistic but I think this is doable. What do you guys think?

[kkoopa]

The argument is not that ugly and it is not that inflexible. It is basically a back-port of the new v8 weak callback API to the old version, with a bit of added smartness and harmonization.

The basic problem is that these are the two APIs from v8's end. On top of that you still need to be able to dispose of the Persistent and Persistent handles are non-assignable in new v8.

New v8:

template<class T, class P>
class v8::WeakCallbackObject< T, P >;

template<typename P>
V8_INLINE void SetWeak(
    P* parameter,
    typename WeakCallbackData<T, P>::Callback callback);

template<typename S, typename P>
V8_INLINE void SetWeak(
    P* parameter,
    typename WeakCallbackData<S, P>::Callback callback);

---

Old v8:

typedef void(* v8::WeakReferenceCallback)(Persistent< Value > object, void *parameter)

A weak reference callback function.

This callback should either explicitly invoke Dispose on |object| if V8 wrapper is not needed anymore, or 'revive' it by invocation of MakeWeak.

Parameters:
object the weak global object to be reclaimed by the garbage collector
parameter the value passed in when making the weak global object

template<class T >
void v8::Persistent< T >::MakeWeak (void * parameters, WeakReferenceCallback callback)          [inline]

Make the reference to this object weak.

When only weak handles refer to the object, the garbage collector will perform a callback to the given V8::WeakReferenceCallback function, passing it the object > reference and the given parameters.

[bnoordhuis]

Let users define the callbacks more freely (free function, static member function, functor, member function, what ever) and then deal with it in NanMakeWeakPersistent(...).

The V8 API only allows you to store a function pointer and a void* (this is true in v0.10 and v0.11 but in v0.11, the void* is a T*.) For member functions, you would have to wrap the target callback in a heap-allocated struct and dispatch it in the weak callback. For non-member functions, you should be able to elide the heap allocation.

[agnat]

Hehe... all them technical arguments about gritty implementation details. Let's see...

Please forget about the difficulties in implementing this, forget about the inside for a moment. Most of these issues are already dealt with in the current implementation. Let's focus on the user interface.

@kkoopa mentioned some shortcomings of NAN_WEAK_CALLBACK: It being a template, keyword static, &c. I want to know why it is a template in the first place, why a declarative macro with that fall from the sky data argument? The best reason I could come up with is that it was deemed to ugly for the user to write:

void callback(NanWeakCallbackData<v8::String, int*> const& data);

... but you tell me. Then, after the decision to use a declarative macro, using a template becomes an attractive choice because you can capture the argument types at invocation time. Again, my own reverse-reasoning. If I'm wrong just tell me...

In my view @kkoopa argument about free vs. static member functions boils down to this: C++ has to many things you can call. All or at least most of them have valid applications in the context of a NAN binding. A very node specific add-on with tight v8 coupling uses static member functions while a binding for an existing C library needs free functions, &c. In fact the most common use case of static member functions is to unpack this and call a member function. It would be nice to cover more of these options.

The problem is all these $CALLABLES are slightly different, either in declaration- or in call site syntax or both (Everybody take cover, because @kkoopa is cocking his Haskell shotgun). That's why a declarative macro will never fit the bill. That's why I came up with the stuff in my first comment.

But again, this is only my view on the subject. Tell me what kind of improvements you had in mind.

[kkoopa]

Yeah, it's mostly cosmetical. I guess it might work without it. You want the arguments captured, yes. And this is WeakCallbackData on new v8:

  444 template<class T, class P>
  445 class WeakCallbackData {
  446  public:
  447   typedef void (*Callback)(const WeakCallbackData<T, P>& data);
  448 
  449   V8_INLINE Isolate* GetIsolate() const { return isolate_; }
  450   V8_INLINE Local<T> GetValue() const { return handle_; }
  451   V8_INLINE P* GetParameter() const { return parameter_; }
  452 
  453  private:
  454   friend class internal::GlobalHandles;
  455   WeakCallbackData(Isolate* isolate, Local<T> handle, P* parameter)
  456     : isolate_(isolate), handle_(handle), parameter_(parameter) { }
  457   Isolate* isolate_;
  458   Local<T> handle_;
  459   P* parameter_;
  460 };

[agnat]

You want the arguments captured, yes.

I disagree. I'm convinced that NAN_WEAK_CALLBACK (responsible for the capturing part) is the problem. It breaks in so many cases. It is also very awkward to put the user into the position to write a (static) template (member) function without letting him know. Even after looking at the definition of NAN_WEAK_CALLBACK it is still not obvious to the average user that he can always mitigate any problem by writing a plain (static member) function. It is very obscure and shows "unexpected behavior" that is very hard to understand for the average user. It would be much cleaner to have users spell out the non-template signature.

Regarding the actual signature I take it that you want to keep it as close to the (new) v8 stuff as possible. I'm wondering a little why that is? I think coming up with own lightweight abstractions is not only successful (NanNew<T>(...)) but also provides a bit of isolation. Isolation against v8 changes and isolation against changes of the inner workings of NAN. Totally revamping NanNew was only possible because there was a little bit of isolation to work with.

[agnat]

At least rename T and P in NAN_WEAK_CALLBACK. ;)

[agnat]

Ok. Nobody seems to have an actual suggestion on how to improve this and my view on the subject wasn't to your liking... Guess it isn't that pressing and we can close it for now.

[kkoopa]

Maybe NAN_WEAK_CALLBACK could be removed, but what should be the consistent way of defining them then? There needs to be some wrapping code on the old side to make the arguments compatible.

[agnat]

Well, since you'd like to keep it close to the v8 API it would be something like:

void someCallback(NanWeakCallbackData<v8::Object, int> const& data);

... or what ever types T and P the user picks. That works on both APIs, new and old or am I missing something?

[kkoopa]

Yes, I guess that would work, as such. But, it would still require the stub to
do type conversion. That is not a problem as such, as MakeWeak can take care
of going through the stub, but it will require access to the type information
at some point too.

On Thursday 20 November 2014 05:45:17 David Siegel wrote:

Well, since you'd like to keep it close to the v8 API it would be something
like:

void someCallback(NanWeakCallbackData<v8::Object, int> const& data);

... or what ever types T and P the user picks. That works on both APIs,
new and old or am I missing something?

---

Reply to this email directly or view it on GitHub:
#218 (comment)

[agnat]

Again, either I don't get the point or you underestimate your doing. One sec...

[agnat]

As I thought. It just works. See above.

[agnat]

The template function NanMakeWeakPersistent does an excellent job of capturing the types. That is the point of template functions. Same thing with NanNew: Capture types using a template function, use template classes to do the "heavy lifting" inside.

[kkoopa]

Good, it does work. The stub I mentioned is _NanWeakCallbackDispatcher. What I would like to achieve is to get rid of the indirect call at https://github.com/rvagg/nan/blob/master/nan.h#L1485 and give the callback function as a template argument and have it inlined into unique _NanWeakCallbackDispatchers based on the function given, without changing the external interface of NanMakeWeakPersistent. This would be slightly more efficient than the indirect call, which cannot be inlined. Based on some reading I did, I believe it is necessary to make a functor of the callback in order to achieve this. Is this somehow possible to do?

[agnat]

Hehe... Ah, I see... 1337 micr0-optzns ala node. Let's worry about that single function call later*. Let's start by removing some unnecessary indirection in the design. I think the version above is a bit more readable. What do you think?

• ...as in "never"

[agnat]

So, now regarding that optimization:

You'd like the user callback to be inlined into what's now called invoke (was NanWeakCallbackDispatcher), right? I really don't think it is worth the effort. The thing is: Any additional template argument will show up in the signature of the callback, which already is quite a mouth full. Calling a function pointer feels like the right thing to do.

[agnat]

Any additional template argument will show up in the signature of the callback

That's probably not true. It probably is doable. However, I still think it is not worth it...

[kkoopa]

Weak persistent cleanup looks good. At least it passes all tests. Avoiding the indirect call would be perfection. I was thinking of something along the lines of http://stackoverflow.com/a/2156899/4214433 using template by value.

[agnat]

Perfection... Perfection is in the eye of the beholder. And since this is your eye and your perfection it is yours to achieve. This also seems fitting since you are in academia and you decided to learn/research this stuff ;)

[kkoopa]

This is not connected to my research, it's a spare time project. There is little research to do in C++ template metaprogramming. It has already been proven Turing complete. Anyway, these changes can only go in the next major version, as they are breaking backwards compatibility.

[agnat]

Just pulling your leg...

I know TMP is turing complete. So is Haskell. Then why do Haskell research? :-P

AFAICT, these changes are not breaking compatibility. The API remains unchanged. I only changed internal names. I hereby invoke:

However, do as you please. I don't mind...

[agnat]

No C++ research... lol

Please tell that David Abrahams, Andrei Alexandrescu et al. Tell that the boost guys. Tell that to the guys who are constantly revamping the standard to reflect the latest discoveries and insights gained in research.

TMP is a great example because its turing completeness is a discovery and not by design. Hence the awkward syntax. It not being invented is also a strength: It is mathematically pure, which in the context of an industry language like C++ makes it a very interesting subject of research.

I understand that it is not your area of interest but to dismiss it as a subject altogether seems a bit arrogant. Just saying...

[bnoordhuis]

TMP is a great example because its turing completeness is a discovery and not by design.

That's not really accurate. Templates were suspected to be Turing-complete right from the start but it wasn't formally proven until years later because really, what's the point? Being Turing-complete doesn't say anything about usability or efficiency. Take Magic: The Gathering; it's Turing-complete but you wouldn't use it to power Crisis.

[kkoopa]

I was not dismissing it as a subject in its entirety. I was merely stating that there was little research value in the particular problem of giving a function as a template argument and getting it inlined in another function. It is more a matter of implementation than theory.

[agnat]

[...] because really, what's the point? Being Turing-complete doesn't say anything about usability or efficiency.

Exactly! Thank you.

Take Magic: The Gathering; it's Turing-complete [...]

Really? Cool. /me gets his cards and builds a fibonacci deck.

I'm not an expert in C++ history. But I think when Bjarne Stroustrup designed them he was thinking only a little beyond container<T>. Later STL implementors started to probe the limits by introducing trait types. Since then it is a dance between compiler guys, researchers and hackers to explore their properties and value. Personally, I've always been fascinated by TMP. If you're interested I just dug out an old experiment of mine: n2o

[agnat]

I was merely stating that there was little research value in the particular problem

Considering your current language skills it still would make an excellent exercise. However, you will discover that the amount of clutter it introduces is not worth the gain... ;)

[agnat]

On second thought the turing completeness of templates is an important point. Because only if they are complete it makes sense to think about templates as compile-time meta programs. Efficiency is more a matter of compiler design and they are getting there.

However, I still agree that just being turing complete proves nothing. INTERCAL illustrated this nicely in 1972.

[kkoopa]

So, everybody, should we remove the not fully proper NAN_WEAK_CALLBACK macro and have users declare their weak callbacks like void someCallback(NanWeakCallbackData<v8::Object, int> const& data);?

While I dislike seeing NAN_WEAK_CALLBACK disappear from an esthetical point of view (it goes well with NAN_METHOD et al.), it has a different structure and simply cannot be made to work exactly the same way as NAN_METHOD et al., so I guess I give it a +1.

[bnoordhuis]

We should have an abbreviation for "no opinion, do what is best". Can I coin NODWIB?

[kkoopa]

Merged as a65ee94