Refactoring SpiderMonkey

Contribution Depot
22

Our apologies for the double correspondence: we first decided to make contact through mail, but then decided to use the forum for more open communication. We also did not mean to imply SpiderMonkey was not functioning properly at the moment in any way.

Thanks a lot for the discussion. Our current plan is as follows: We will write tests for the current Serialization behavior in order to capture the current functionality in an automatic and reproducible fashion. We will then create another version of this behavior using Java’s built-in Serialization functionality, and assess its performance in relation to the current behavior. Our plan is also to report these findings (positive or not) to you, and maybe even discuss a pull request if the findings are positive. We will try to contribute the tests we wrote anyway.

We think it could be useful, as it could shorten the SpiderMonkey library, and less code generally means less potential defects. It seems like it could also shorten client code by a small bit, eliminating the need to register classes for serialization. We realize there are potential pitfalls, as the Java built-in serialization seems to bring large overhead (as mentioned here and here).

In our research so far two questions sprung up that we have been unable to find an answer for so far, do you mind if we use this topic for our findings? If so, please tell us what a better location would be.

  1. Is it intended behavior that you are unable to add more (TCP) channels when not specifying a UDP channel? You can create a Server with only a TCP channel. This means the channels List will have only one channel in it. Trying to add a channel using addChannel will result in an IllegalStateException (line 149 in DefaultServer.java). If it’s not, should we create an issue?

  2. Why are the Maps in the FieldSerializer static? It seems that private static fields are shared among multiple instances of the same class, but we didn’t think different FieldSerializers should share their saved fields and constructors. We stumbled across this when trying to execute new FieldSerializer() in a test: it gave a NullPointerException for savedCtors on line 62 in FieldSerializer.java, which we found to be very odd. Removing the static modifying removed the error.