Closed weavejester closed 11 years ago
This looks great! Merging to test locally.
I tested this some more and unfortunately it looks like tests are variable. Sometimes they work, but occasionally they don't.
I suspect it depends on the order Java loads the classes in. Sometimes IRecord is preferred over IPersistentMap, and it works okay, but sometimes it loads it the other way round, and records are serialized as maps instead. I suspect this can be solved by using APersistentMap instead. I'll test and create a patch.
Yeah, picked that up on my end as well. Problem actually seems to be the IPersistentCollection dispatch. Not sure what the underlying cause is since it seems to be variable.
In any case the IPersistentCollection is vestigial and can be removed, which seems to solve the problem.
Give me a few mins to push a new update with a few other changes - just busy tracking down the cause of a performance regression.
Thanks a lot, and sorry for the bug in the record pull request. I should have realized this might have been a problem, and ran some more tests.
Thanks a lot, and sorry for the bug in the record pull request.
Not at all. It's an obscure problem, and easy to miss with the variable test results. These were awesome PRs - much obliged.
This patch allows records to be serialized and deserialized by Nippy. The serialization scheme is more performant than the EDN reader, but less performant than a custom type due to the use of reflection to reconstruct the record type. The record class must be available on the classpath in order to deserialize the record.
Because this patch adds a explicit record serialization, it also effectively fixes issue #24, without needing to change any of the existing dispatch types.