Using private fields to store properties, or not ;-) about modelsbuilder.original

[YodasMyDad]

I'm also interested in this. We still map manually because of performance and flexibility. I looked at Ditto and was thinking about using it, but open to other ways.

[zpqrtbnk]

Imagine you have a Google Maps property type with an associated property value converter, that would return Location objects, each object having a Latitude (float), Longitude (float) and Zoom (int) properties. Internally, the property type stores a JSON string in the database, eg {lat:47.222,lng:3.5598,zoom:3}.

When you get content.Location from a model, it returns a Location object, and you can write things such as @Model.Location.Latitude. Internally, all the model does (at least those models built with the models builder) is define a property as:

public Location Location
{
  get { return this.GetPropertyValue<Location>("location"); }
}

So really, whether you use a model or not should not make a difference, perfs-wise: it all comes down to the performance of GetPropertyValue<TValue>(string alias). Let's see what that method does:

1. get the property itself ie content.GetProperty(alias)
2. get the property value ie property.GetValue()

In (1), GetProperty does:

return Properties.FirstOrDefault(x => x.PropertyTypeAlias.InvariantEquals(alias));

That could be optimized by using a dictionary of some sort, and it definitively has a cost, but probably a marginal cost related to all the things that take place when rendering a page.

In (2), the value is obtained from the raw database value, via conversions specified by the property value converter. There are plenty of Lazy<T> in there that ensure that the conversion runs only one, for that property instance.

So, assuming you write, in a view:

<div>@Model.Location.Latitude</div>
<div>@Model.Location.Latitude</div>
<div>@Model.Location.Latitude</div>

Then on the first row, (1) runs and then the conversion runs. On the next rows, only (1) runs (so you take the lookup perfs hit) but the conversion does not run again.

Finaly, because published content objects are not (for the time being) cached outside the current request, this is what will happen on every request, ie we do not share the conversion result between requests (that's a thing that the "new cache" should fix).

And now... back to your questions...

No, the converters would not run each time but only once per request. But anyway I would advise against caching IPublishedContent instances outside of the current request for the time being, as the cache is not designed to support that and "anything" could happen.

Yes, a private field in the model would somehow bring a perfs gain, although that field would need to be lazy in a way so that the conversion runs only if you actually need the property value - my personal feeling is that the gain would be marginal but I can be wrong.

And so, my (personal, again) opinion at the moment is that adding private fields on top of what we already have would be premature optimization - but I'm happy to discuss it!

[kipusoep]

Thank you for your comprehensive response 👍
Just to make sure, the implementation I'm thinking about looks like this:

public Location Location
{
  get
  {
    if (_location == null)
      _location = this.GetPropertyValue<Location>("location");
    return _location;
  }
}

Location _location;

I might try the current implementation and this implementation and do some performance testing to find out for sure, if that's ok with you?

[zpqrtbnk]

What if GetPropertyValue<Location> can return null? ;-) You'd need a Lazy of some sort in that case. Just being picky here: sure, go ahead, I'm quite interested in the perfs testing results! There is going to be a gain, for sure, but is it going to be significant?

[kipusoep]

Ah yes, ofcourse, lol!
But if there's a performance gain for sure, there's no reason to not implement it right? ;-)
Would it be hard/time consuming? Looks like a small template adjustment to me, but I haven't build this ;-)

[zpqrtbnk]

Like, implement it in the builder? Should not take that long. And then it's balancing the cost of creating Lazy instances vs. doing the lookup each time. Could even make it an option anyways. Mmm... will look into it (changing the issue from question to enhancement).

[kipusoep]

:-D Looking forward to start using this project, looks really promising!

[zpqrtbnk]

As I'm working on the NuCache I realize that this is not going to make it. See, the NuCache caches the contents once and for as long as it is not modified. But, a property value may reference another content and therefore you need to let the Core manage the cache refreshing. If you cache in the model, you may optimize, but you're going to create inconsistencies.