Discussion about lazy val definitions

[cornerman]

Currently, domtypes defines dom attributes, properties, styles, events and elements as lazy val.

Now, we use domtypes in OutWatch, where most of the things we are constructing are builders. For example, the attribute contenteditable is an AttributeBuilder to which you can assign Boolean values. So you would write something like contenteditable := true. This is evaluated into Attribute("contenteditable", "true"). Ideally, our code should be compiled into the evaluated Attribute directly. Now, our AttributeBuilder already uses inline annotations to inline most of the construction part, but the compiler cannot inline the whole expression, because contenteditable is defined as a lazy val.

I would like to evaluate whether we can use @inline def instead of lazy val for the definitions in domtypes. What do you think? What kind of tests/evaluations would you like to see?

[raquo]

Hm.

I'm confused when you mention Attribute. I assume you mean this one?

type Attribute[T, _] = helpers.AttributeBuilder[T, Attr]

But then, contenteditable := true is not an AttributeBuilder, it's an Attr.

So if you change attr definitions from lazy vals to defs, you would be creating new instances of AttributeBuilder whenever you refer to attr in attr := value, attr <-- valueStream, etc.

That is... a lot of short lived allocations that currently don't happen because those builders are cached by lazy val. I don't think the def being @inline would let you avoid all those allocations... Am I wrong?

What I'm saying, I don't really see how you can inline attr := value all the way to Attr("attr", value) without instantiating AttrBuilder("attr"). And if you do want to pay the runtime cost of all those extra instantiations, then to what end? What do you gain instead?

[cornerman]

Thanks for the response! I mentioned Attribute as an example, sorry for not providing enough context. But It is exactly about Attribute being an AttributeBuilder which basically looks like this:

case class Attribute[T](key: String, value: T)

@inline class AttributeBuilder[T](key: String) {
  @inline def :=(value: T): Attribute[T] = Attribute[T](key, value)
}

Now if I write something like the following, I would actually allocate an AttributeBuilder and then construct my Attribute.:

lazy val builder = new AttributeBuilder[String]("contenteditable")
val result = builder := "true"

The above compiles to:

val builder = new AttributeBuilder[T]("contenteditable")
val result = Attribute[String](builder.key, "true")

If I now instead define my builder as an inline def, I will never actually allocate an AttributeBuilder, but the compiler (escpecially the scala-js optimizer) will adhere to the inline annotations and compile everything down to the Attribute construction without the builder in between:

@inline def builder = new AttributeBuilder[String]("contenteditable")
val result = builder := "true"

The above compiles to:

val result = Attribute[String]("contenteditable", "true")

Now, there will be constructions where you cannot avoid allocating a builder because inlining would not work. If for example I had a non-inlining function returning an AttributeBuilder and then use that builder somewhere else. But in my experience, most of the code directly applies := the builder and therefore never needs an allocated builder.

So, what I would gain is hopefully less runtime overhead for constructing nodes in OutWatch, because intermediate builders can be inlined for the most part. Currently the lazy val blocks inlining and is the reason why I need to instantiate them in the first place. This gets even more interesting for building more complex modifiers or the whole node. Given, I would have the html strings like contenteditable all over my compiled code.

I guess, we would want to test how reliable inlining is to be sure it makes sense. Furthermore, this will not work that well on the JVM, because scala-js does a lot more for inlining functions than just the scala compiler.

[raquo]

Interesting, I didn't realize classes too could be inlined in ScalaJS. I'm guessing the behaviour of that is somewhat similar to value classes (extends AnyVal), but allows for classes with primary constructors having multiple parameters like (key: String, codec: Codec[...])? Is there any documentation anywhere regarding when the @inline class will have to be instantiated? Value classes have specific rules to avoid instantiation that even though documented are annoying to comply with.

--

Assuming class inlining works sufficiently well, let's see what we can get out of it:

• Avoid instantiating AttrBuilder. With lazy vals, those would be instantiated on demand and then cached. Depending on the project we're talking about I'd guess... 100 builder instances or so.
• On access, avoid if (lazyValCached) lazyVal.attrKey in favor of inlining "attrKey" strings for := and <-- use cases. Accessing a string is a bit faster than accessing a field, but then you have those strings copied across the bundle. Gzip should be able to compress that fairly well I think, but pre-gzip bundle size is still important for JS parsing time when the page loads.
• Use cases that aren't simple := or <-- invocations suffer from an extra instantiation. I agree that those would be rare though.
• Since attribute builders are not cached anymore, comparing them using == wouldn't work anymore. We would need to make them into case classes, which would likely be additional overhead, or compare string keys instead. Neither is good but this case should also be rare.

Overall, intuitively, this seems like a small and controversial net gain that's dependent on fragile and probably undocumented inlining behaviour, so I'm skeptical about this approach.

If you have a build of dom-types with inline defs, have you tried running your project with it? How does it affect your un-gzipped fastopt FULLopt bundle size? Is there a noticeable runtime performance benefit?

[cornerman]

Sorry for taking so long with my resonse!

Is there any documentation anywhere regarding when the @inline class will have to be instantiated?

As far as I know there are reasons why things cannot be inlined like returning an inline class in a method that itself is not inlined. But I agree, as of now, this feature is definitely under-documented. Though, inlining in scala-js works really well for me, in my experiments it was much more reliable than AnyVal.

Another cool thing you can do with inlining is unrolling lambda functions. For example:

@inline def doSomething(f: String => String): String = f("test")

val result = doSomething(_ + "!")

This can be compiled to:

val result = "test" + "!"

Inlining functions can be really nice for tight loops and building zero-cost abtractions.

Accessing a string is a bit faster than accessing a field, but then you have those strings copied across the bundle. Gzip should be able to compress that fairly well I think, but pre-gzip bundle size is still important for JS parsing time when the page loads.

We need to do benchmarks on this and compare the output of fullOpt as you proposed -- otherwise we just do not know. I imagine using Strings directly would be the more performant than the lazy val construction.

Since attribute builders are not cached anymore, comparing them using == wouldn't work anymore

Why do you need equality on the builders? Relying on the builders to be cached for proper equality seems a bit fragile to me anyhow. I think having a proper equals method is the way to go. What if someone creates a builder without using the provided lazy vals? Or what if someone extends the trait with properties multiple times?

If you have a build of dom-types with inline defs, have you tried running your project with it? How does it affect your un-gzipped fastopt FULLopt bundle size? Is there a noticeable runtime performance benefit?

I have a branch and tried to reduce the generated code for creating virtual nodes in outwatch, but did not have time to investigate this in more detail, yet. I will keep you updated on this one - just not sure when I will find the time.

[raquo]

I've been looking for more info of how inline classes work, here are a couple explanations:

https://gitter.im/scala-js/scala-js?at=561531aece6e633c4518d5a4
https://gitter.im/scala-js/scala-js?at=57e2fb8f27a8458f7f34f3e7

So if I understand correctly, all usages of a given instance of inline class need to be contained within a single method in order for that instance to be inlined. So, a method can not return an inline class – that class would not be inlined then. Unless the method itself is inlined. Then the same criteria applies to the method that calls this method. So... perhaps if we inline enough methods and classes, we'll get the original class inlined as well.

However, looking over Laminar code, I suspect there will be a tradeoff between reducing type safety and increasing bundle size. Essentially to deal with methods into which inlined classes escape we can either change their type signatures to receive individual fields of those inlined classes to avoid them escaping, or we can inline those methods to expand the inline boundary, but that would mean code duplication in the bundle. Hope that makes sense. You can see what I mean in DomBuilder's Setter class and JsHtmlElementApi.setHtmlAttribute methods. I think both of those would need to be inlined or dealt with in some other way, but I'm not sure.

I'm curious to hear if your inline def branch produces the desired results whenever you have time to look into that.

[cornerman]

Nice, thank you for the links to the gitter conversation!

However, looking over Laminar code, I suspect there will be a tradeoff between reducing type safety and increasing bundle size.

I agree that optimizing this properly is a thin line, because you really have to take care that everything in between is inlined. The intended optimization can break when you add new code or more abstractions. That is the sad part of it :/

You can see what I mean in DomBuilder's Setter class and JsHtmlElementApi.setHtmlAttribute methods. I think both of those would need to be inlined or dealt with in some other way, but I'm not sure.

Ah I see, you are building these setters as your Modifiers. My first intuition would be to not inline the setHtmlAttribute function, because that seems like it would duplicate too much code in the end. As I understand, the action in the setter could, e.g., be the setHtmlAttribute function. I think, it is fine if the Modifier class and its methods are not inlined. Because you really want to concentrate on inlining the builders (like ReactiveProp).

[raquo]

The thing is, I do need to inline both the Setter class and the setHtmlAttribute function in order to avoid instantiation of ReactiveHtmlAttr, because currently an instance of ReactiveHtmlAttr escapes the original method scope first to Setter and then to setHtmlAttribute.

Alternatively, I would need to change setHtmlAttribute signature to accept attrName and encodedValue instead of ReactiveHtmlAttr, but that is almost like inlining it anyway, as the only thing setHtmlAttribute would still be abstracting at this point would be the if condition, and other methods like setProperty wouldn't even do that, they would become completely useless.

Also I'm not sure if changing the signature like that would even work for inlining purposes, as setHtmlAttribute is also used in ReactiveHtmlAttr's <-- method. That method would probably also need to be reworked, replacing new Modifier {} with a new, inlined Setter-like class.

But there's more. When we build div(key1 := value1, key2 := value2), we get div(Setter(...), Setter(...)). Assuming setHtmlAttribute is taken care of and those Setter-s are inlined, the div() call itself (TagSyntax.apply(modifiers: Modifier[...]*) in Laminar) also needs to be inlined because otherwise Setter-s would escape method scope and will be instantiated.

But TagSyntax.apply takes a Seq of Modifiers (Setter is a Modifier, remember) using VarArgs and then foreach-es over it. If Setter instances are inlined and not actually instantiated, how would that even work? I think the answer is probably it won't, because Setters have already been quietly instantiated when a Seq was constructed out of them to be passed into TagSyntax.apply. Because Seq itself (or well, its concrete subclass) is not an inline class.

And remember, we need Setter inlined because otherwise the original thing we wanted to inline, ReactiveHtmlAttr, would escape method boundary and would not be inlined.

The more I look into this, the more fragile and impossible this seems, unfortunately :| This basically needs a lot of painful testing to verify that inlining is actually happening, let alone what effect it has on bundle size and performance.

[raquo]

Copied from #86:

Scala.js 1.11 (news) now removes unused fields.

If I understand correctly, this means that we can have e.g. val div = htmlTag("div") instead of it being a lazy val, and if you never use div-s in your app, div won't be included in the JS bundle, similar to how it works today with lazy vals.

This would be great for us because having a hundred lazy vals for all the tags and properties that you use obviously comes with some overhead, and even if it's not really noticeable, it would be great to simplify.

However, we need to test whether the Scala.js optimizer is smart enough to recognize our implementations of def htmlTag as "pure", and thus removable, otherwise we won't win anything. For Laminar the implementation instantiates a class HtmlTag which inherits from a trait with two public val-s, the constructor doesn't do anything else other than set those vals. For Outwatch and other consuming libraries, I'm not sure, but I suspect it's something similar. I'm hoping all our implementations are pure enough.

Since Scala DOM Types 17.x, this decision is now on the authors of libraries like Laminar, Outwatch, and Calico – if anyone tries any non-lazy val approaches, please let me know how it went.