Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serializer should cache results from runtimeType.toString() #637

Open
davidmorgan opened this issue Apr 18, 2019 · 7 comments
Open

Serializer should cache results from runtimeType.toString() #637

davidmorgan opened this issue Apr 18, 2019 · 7 comments
Assignees

Comments

@davidmorgan
Copy link
Collaborator

Since runtimeType.toString() may be slow, we should cache the mapping back to the base type for use later. (To be confirmed by benchmarks, of course).

@mkustermann
Copy link

@davidmorgan Do you think this optimization could be implemented? We seem to see this on profiles, especially app startup.

@mkustermann
Copy link

Slightly orthogonal to this issue: Would it be possible to avoid relying on string representation of runtime types (which we don't really promise anything about - e.g. obfuscation may change them)? There seems to be support for an explicit wireName - could it be enforced to use such names always instead of leaning on runtime types?

@davidmorgan
Copy link
Collaborator Author

Yes, it should be possible to do this soon; I'm going to be migrating the generator to null safety, it will be convenient to make other small changes at the same time.

Re: the slightly orthogonal issue ;)

Obfuscation doesn't matter as long as it's stable over one run of the VM. The only other property we need is that if there is a < in the result then stripping off everything up to that point gives you a way to identify the base type without generics.

As far as I know this is the only fast way today to map a generic type back to the base type at runtime, e.g. to notice that a Foo<int> is a Foo and a Bar<int> is a Bar. The alternative is a sequence of is checks which does not scale well. We need this because any Foo<X> must be dispatched to the Foo serializer. I would be happy to replace with something better if we can find something that works for VM, Flutter and Web :)

Thanks.

@mkustermann
Copy link

mkustermann commented Apr 25, 2022

👍 for looking into this!

If all objects that can be serialized have common base classes / interfaces they implement, would something like this be feasible:

abstract class Serializable {
  Type get rawType;
}
class Foo<T> implements Serializable {
  Type get rawType => Foo;
}

?

Then the cost would be one method call that returns a constant. This constant can then be used as keys in maps, etc. No need to go via intermediate Strings. (or instead of Type get rawType => Foo use String get rawTypeAsString => "Foo")

@davidmorgan
Copy link
Collaborator Author

Unfortunately there's no common base class.

That would have problems of its own: for example, primitives don't implement it, so you can't refer to all serializable types statically.

It's really something that needs just a liiiitle bit of language support ;)

@davidmorgan
Copy link
Collaborator Author

Here's a PR: #1132

@mkustermann do you have any handy benchmark we can check it with please? I always like to check performance improvements actually work ;)

@mkustermann
Copy link

@davidmorgan will forward some information offline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants