marcglasberg / fast_immutable_collections

Dart Package: Immutable lists, sets, maps, and multimaps, which are as fast as their native mutable counterparts. Extension methods and comparators for native Dart collections.
BSD 2-Clause "Simplified" License
218 stars 30 forks source link

Use unsafe immutable collections for JSON parsing #54

Closed Reprevise closed 1 year ago

Reprevise commented 1 year ago

When creating collections from JSON, the package is creating copies of the original collections when it doesn't have to.

For example, the IMap.fromJson constructor code:

factory IMap.fromJson(
  Map<String, Object?> json,
  K Function(Object?) fromJsonK,
  V Function(Object?) fromJsonV,
) =>
    json
        .map<K, V>(
            (key, value) => MapEntry(fromJsonK(_safeKeyFromJson<K>(key)), fromJsonV(value)))
        .lock;

Considering the fact that a new map is being created via .map, wouldn't it be safe to use .lockUnsafe here? (And in the other constructors IList and ISet as well.)

marcglasberg commented 1 year ago

Absolutely. Good catch! I will fix this.

marcglasberg commented 1 year ago

Done. Note just the IMap had this problem. IList and ISet where created differently. Thank you.

Reprevise commented 1 year ago

@marcglasberg Can you explain why IList and ISet don't have this issue?

Both IList.fromJson and ISet.fromJson use their default constructors to create their respective collections which AFAIK does create copies unlike ISet.unsafe and IList.unsafe.

marcglasberg commented 1 year ago

IMap is creating a map first, and then locking it.

But IList and ISet are not. For those you have an Iterable, and then you lock it directly. The Iterable is lazy, it will be used internally when creating the IList/ISet. No intermediary mutable List or Sets are created in this case.