Add tests for runtime instantiation of picklers

[jvican]

• This is an important use case for distributed programming frameworks
  that scala pickling should cover: the ability to create instances of
  picklers and unpicklers from their classnames.
• Use staticOnly since we want best performance and elide the type of
  the wrapped value in Container.
• These tests are not passing and give NPEs since when we reflectively
  instantiate the picklers and unpicklers the implicit values passed in
  the scope are null (sun.misc.Unsafe is the one instantiating them, the
  by-default java newInstance fails to find a zero-args constructor).
  Also, we get NPEs when looking up FastTypeTags that have been
  already generated before.
• Although this can look like a reflection limitation, I think that it
  is related to the way the picklers/unpicklers are generated or
  registered, and even how the FastTypeTags are created; and therefore
  we should try to find a way to fix it.
• There are three classes of tests: one with the default generated
  picklers/unpicklers for Container, other one with custom
  picklers/unpicklers that rely on the values in the implicit scope
  (these may never work because of reflection but I add them to make clearer
  the point that they don't work) and the last one that uses implicitly
  calls for getting the tags, picklers and unpickler and thus should work
  since they are just looking up in the registry (in theory).

[jvican]

Do you have an idea of what's going on with the FastTypeTag @jsuereth? I'm especially interested in making work the first test case and the third one, the second one is just there to show it doesn't work. To me, it looks like implicitly[FastTypeTag[T]] in line 106 should work. Any suggestion would really help Josh. And, by the way, thanks for the latest refactorings that you've done, they're fabulous.

/cc @phaller @heathermiller

[jsuereth]

@jvican I'll look into this now. I think you caught a bunch of the issues I've been stumbling through with my FastTypeTag refactorings (which I was making right before diving into runtime pickler changes).

But, to put it bluntly: You cannot safely reflect over arbitrary types in scala reflection and get the actual physical layout of the object, I think we'll have to revert to using java-reflection in a lot of these scenarios.

In fact, one of my goals (next) is to clean-up runtime pickler generation so that:

• It uses the macros to generate source code of pickler/unpicklers and interprets them on the fly (i.e. require the compiler.jar for runtime reflection).
• When the compiler is unable to generate a static pickler we revert ALL the way down to pure java reflection, rather than scala reflection (which is not designed to give us enough of an idea of physical layout).

Now, for the implicit argument issue, that sounds just like a straight up bug we can fix :) I'll dive in shortly.

[jsuereth]

why are we doing this?

[jvican]

I have a use case in which I need to pickle an entity whose type parameter is unknown (imagine that a client sends a message Msg[T] to a server and that server has to reply with Reply[T]). The only approach to deal with this is to pass the pickler class name of Reply[T] into Msg[T] since in the server we don't know the type of T and there's no way we can get it from the unpickled Msg[T]. This is exactly what I'm trying to replay here.

[jvican]

BTW, I'm also doing this because I want to avoid using runtime pickling, I only want static picklers. As I'm sharing the jars between the server and the client, there's no problem.

[jsuereth]

@jvican FYI, in "hybrid" mode, you can register static picklers for runtime lookup, but they're still static picklers.

The PicklerRegistry supports an option to REMOVE the runtime pickler generation code. This is "hybrid-static" picklers, vs. "full hybrid" picklers.

Fully static is where you can know the types of everything when you pickle/unpickle and there's no ambiguity in your ADTs. "hybrid static" is where you pre-register all known types of picklers, but there is still runtime lookup (and runtime failures) for some of your ADTs. "hybrid static" allows you to extend your "protocol" while also keeping some static safety. It's meant to be a blend of the two approaches that lets you sacrifice the minimum of static safety.

In any case, even though I'd argue we should be able to find you an alternative mechanism here, I'll see if I can get the tests working. They should work, and you've uncovered another hole in our test suite, where this should be caught/tested.

[jvican]

So I guess that I'd need hybrid-static picklers since they match my exact use case. The thing is that I can't register static picklers for runtime lookup because that registering is actually done at runtime (unless you're inlining the code responsible of that in the initialization of all the scala pickling machinery using macros). As it's done at runtime, the server (which is an independent app than the client) will never execute that code even though they share the same jars. Is there any way to actually register those static picklers automatically without me doing anything?

[jsuereth]

I was just talking with @non about this. It may actually make sense for you to serialize the capture class + spore class over the wire, and create your own classloader to pull in the spore. I did this previously (see: https://github.com/jsuereth/serializable/blob/master/src/main/scala/serialize.scala#L7 for serializing interpreted classes).

I think with spores we could amke this pretty simple/elegant.....

[jvican]

That's actually what we're doing, although we don't use a ClassLoader. Is there any benefit from using it? It sounds to me like I have the same problem than before: I can't generalize this mechanism for both client and server (because the client won't execute the same operations as the server, they don't share any logic, and therefore I can't make them load the pickler classes when they boot up). In this concrete case, the client will serialize something and maybe load it into the classloader, but the server won't have anything in that classloader when unpickling because they're independent).

There's also another failed workaround that I tried before and I didn't tell you. My line of reasoning was the following: as I know that the reference to other implicit unpicklers (the captured unpickler, in this case) will be null when I create the unpickler reflectively, what I do is to instantiate first the captured unpickler using its classname and then set it as a field in the spore unpickler. The problem is that it doesn't work because at the same time the captured unpickler (let's say it's of type Tuple2[Int, String]) will depend on the unpicklers Int and String. When I tried it, I still got a NPE, and maybe in this case could be fixed because they are primitive types, but if the users decide to capture a more complicated type and give custom picklers/unpicklers, it won't work.

[jvican]

Oh, wait, I think I get now what you mean... You're actually serializing the bytes that represent the class (sorry, I miss that detail). That's a great hack, darn it. However, this solution would be the least one I'd like to use since 1. it considerably increases the size of the exchanged messages among the nodes of a distributed system, and 2. It's a very hacky solution that creates an important security hole because you're loading picklers/unpicklers on the fly.

Wouldn't it be possible to have a sort of dictionary of class names and custom picklers/unpicklers that is populated when the scala pickling initializers are executed?

[jvican]

Thanks a lot for chiming in @jsuereth. Why do you think that we could solve the problem with Java reflection? Are we not already using it in the getInstance method? I guess that the difference is that we're creating an instance for a static pickler/unpickler that we have the guarantee it's been generated statically.

Also, if we solve the problem getting the FastTypeTag via implicitly in the body of the picklers/unpicklers, I presume that we don't care if the object layout cannot be reconstructed since we have another way of getting hold of them. Or... perhaps did I misunderstand you?

[jsuereth]

Ok, this makes more sense now. You're creating a pickler using "unsafe new instance", not even calling a default constructor. You'll actually have to PICKLE the pickler to make sure all of its state is safely written/restored, or create something which can look it up by string.

The NPE is actually perfectly expected since you're unsafe allocating anew instance, and then calling methods on that instance. I don't expect this to change, at a minimum you'd still have to call the default constructor after "new instancing".

In any case, I think you want to look at how Any{Un}Pickler actually works. I still have some refactoring to make it cleaning, but follow the trail of logic here. AnyPickler FIRST looks up any registered static picklers, then uses them. You should be able to register your picklers with the runtime lookup and "AnyUnpickler" the values later.

Caveat: I'm still working out all the kinks to make this 100% seamless.

[jvican]

Well, the problem is that there's no default constructor in Pickler/Unpickler and therefore it fails to execute the normal java getInstance.

Yes, as I said in my commit message the NPE does perfectly make sense (in the second test case), what I'm trying to do is to find a solution for the first and third case (in the third test case, even if we instantiate an unpickler and the fields are null, every implicitly call should return the actual objects I'm asking for, and they don't).

It'd fantastic if we can make this work. Aside from this, I'll look into how AnyPickler works more carefully. Still, I have my doubts about registering my picklers. In my previous comment, I explain you why. If I'm wrong, please let me know! 😄 But basically, i think that the registration of picklers/unpicklers should be done by scala-pickling any time a Pickler/Unpickler is generated. Note that I'm developing a library and thus I'd like to avoid forcing the final user who will eventually write the business logic of the clients and servers touch anything related to scala pickling and make this whole process more transparent.

[jvican]

BTW, good to know about Any{Un}Pickler, it's a good alternative, although the idea was to elide all the types. It's a pity that we can't do that when we generate both pickler/unpickler since we have all the type information there and we could avoid wasting one field of the message in the type.

[jsuereth]

I actually agree this should happen. More importantly, when you statically generate a pickler, you should do a quick runtime check to see if a previously generated static pickler was created rather than new-ing a brand new class to handle it all over the place. These are all things I hope to slowly improve for "hybrid" mode, which I think has been the weakest part of pickling mostly just due to lack of time to work on it.

for now, check our "AutoRegister" which I had hoped to add as a part of all statically computed picklers (with some kind of disable switch),albeit we can't be quite as naive as that trait is.