dnrajugade / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

What should happen when serializing a view collection of an *immutable* collection/map/etc.? #1145

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I am writing an application which exposes an interface using RMI. One of the 
methods of this interface returns ImmutableSet<String>. When this method is 
invoked I receive a NotSerializableException. After some investigation I 
discovered that the ImmutableSet was originating from a call to 
ImmutableMap.keySet() invoked on an ImmutableMap whose values are not 
serializable. Looking at the source for ImmutableMapKeySet, it's clear that 
when serialized, the entire original map is serialized as well.

This seems like it could have negative performance characteristics as well as 
the issue I described above. Maps are frequently made up of small keys and 
large values. If the values are serializable, this could easily result in far 
more data being transmitted than is expected or required.

Due to the restricted access on many of the classes in guava, and the 
reluctance to actually copy an ImmutableSet (both decisions which I understand 
and support), the work around for this problem is more confusing and fragile 
than one might expect. I have added the following methods and call 
fixImmutableSet() on any ImmutableSet objects returned:

{{{
  // Have to use class names because the classes are not visible.
  // These class names may change in future versions of guava without notice.
  private static final ImmutableSet<String> SAFE_IMMUTABLE_SET_NAMES = ImmutableSet
      .of("com.google.common.collect.RegularImmutableSet",
          "com.google.common.collect.SingletonImmutableSet",
          "com.google.common.collect.EmptyImmutableSet");

  private <T> ImmutableSet<T> fixImmutableSet(final ImmutableSet<T> set) {
    final ImmutableSet<T> safeSet;
    if (!SAFE_IMMUTABLE_SET_NAMES.contains(set.getClass().getName())) {
      safeSet = ImmutableSet.copyOf(set.iterator());
    }
    else {
      safeSet = set;
    }
    return safeSet;
  }
}}}

Original issue reported on code.google.com by wes...@cutterslade.ca on 12 Sep 2012 at 3:26

GoogleCodeExporter commented 9 years ago
From what I can tell, I'm pretty sure that 

private <T> ImmutableSet<T> fixImmutableSet(final ImmutableSet<T> set) {
  return ImmutableSet.copyOf(set);
}

will actually work just fine here.  The keySet of an ImmutableMap is marked as 
a "partial view," and will actually get a full copy, but the "pure" 
ImmutableSet implementations -- including Empty, Singleton, and Regular, as 
you've mentioned -- will be passed through without being copied.

Original comment by lowas...@google.com on 12 Sep 2012 at 3:56

GoogleCodeExporter commented 9 years ago
I have the vague feeling that I'm forgetting something, but this seems wrong to 
me.  For a *mutable* collection, there's no good answer to whether serializing 
a view should serialize the whole backing data or not, and that's a large part 
of the reason why virtually none of our mutable collection views are 
serializable. But here? I can't think of any advantage to serializing the whole 
backing map. Serializing the key set ought to serialize only the keys and when 
you deserialize it you get a RegularImmutableSet out the other end.  Am I 
missing something?

Original comment by kevinb@google.com on 12 Sep 2012 at 10:06

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:13

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:18

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08