Dismantle instanceof checks

[frankiesardo]

As noted in this benchmark writeValue slows down Parcelable serialisation. Using the type of the field to emit the appropriate method call should increase the speed of ~ 2X and would be a tactical move to later tackle #7

[PaulWoitaschek]

Why would speed that creation? I just checked the source and the current implementation looks like this:

    public final void writeValue(Object v) {
        if (v == null) {
            writeInt(VAL_NULL);
        } else if (v instanceof String) {
            writeInt(VAL_STRING);
            writeString((String) v);
        } else if (v instanceof Integer) {
            writeInt(VAL_INTEGER);
            writeInt((Integer) v);
        } else if (v instanceof Map) {
            writeInt(VAL_MAP);
            writeMap((Map) v);
        } else if (v instanceof Bundle) {
            // Must be before Parcelable
            writeInt(VAL_BUNDLE);
            writeBundle((Bundle) v);
        ...

so the part for serializable is

                writeInt(VAL_SERIALIZABLE);
                writeSerializable((Serializable) v);

What would be optimized here?

Another thought I had is about enums. If I get it correctly the current implementation passes enums by passing it as Serializable. But wouldn't as enums are per definition singletons - the approach to pass them by String name = enum.name() and restore them by EnumType.valueOf(name) be a way more efficient way?

[frankiesardo]

The optimisation is to avoid emitting writeValue and use instead writeString, writeInt etc. instanceof checks are quite expensive it seems

[PaulWoitaschek]

I tried to dig into this a little bit. I don't know too much about velocity but this is a first approach (which works) I came up with.

I don't really get how it produces the whitespace etc but this one works. What it does is it checks the types and starts replacing String and long with their representation. This way many more types could be replaced.

So this is what it produces:

  @Override
  public void writeToParcel(android.os.Parcel dest, int flags) {
    dest.writeString(name);
    dest.writeLong(id);
    dest.writeValue(heightType);
    dest.writeValue(addresses);
    dest.writeValue(friends);
  }

  private AutoParcel_Person(android.os.Parcel in) {
    this(in.readString(), in.readLong(), (HeightBucket) in.readValue(CL), (Map<String, Address>) in.readValue(CL), (List<Person>) in.readValue(CL));
  }

Do you think this is a wise approach to follow? How to I get the

Here comes the autoparcel.vm

  @Override
  public void writeToParcel(android.os.Parcel dest, int flags) {
#foreach ($p in $props)
  #set ($type = $p.getType())##
  #if($type=="String")##
  dest.writeString($p)##
  #elseif($type=="long")##
  dest.writeLong($p)##
  #else##
  dest.writeValue($p)#end;
#end
  }

  private $subclass(android.os.Parcel in) {
    this(#foreach ($p in $props)
    #set ($type = $p.getType())
    #if($type=="String")in.readString()##
    #elseif($type=="long")in.readLong()##
    #else
      ($p.castType) in.readValue(CL)##
    #end
    #if ($foreach.hasNext),##
    #end
    #end);
  }

[frankiesardo]

While I appreciate your effort, the library will undergo a major refactoring soon to catch up with the latest AutoValue extensions. I'm unlikely to merge anything before that and most things will be unmergeable after that. I'll resume this work after 1.0 release.

[PaulWoitaschek]

Yeah no problem. I mostly wanted to learn something new.

And I did only parcel the 2 types to see what you are thinking. But it proves that this works so later something like this can be integrated.