tc39 / proposal-class-fields

Orthogonally-informed combination of public and private fields proposals
https://arai-a.github.io/ecma262-compare/?pr=1668
1.72k stars 113 forks source link

[[Define]] semantics causes libraries to lose performance. #320

Closed trusktr closed 4 years ago

trusktr commented 4 years ago

Class fields [[Define]] semantics causes libraries to take routes of less performance in order to mitigate the overriding non-inheriting nature of class fields.

For example, this is old MobX code with [[Set]] semantics:

class Foo {
  foo = 123
}
decorate(Foo, {
  foo: observable,
})

class Bar extends Foo {
  bar = 456
}
decorate(Foo, {
  bar: observable,
})

class Baz extends Bar {
  baz = 'hello'
}
decorate(Foo, {
  baz: observable,
})

const baz = new Baz

autorun(() => {
  console.log(baz.foo, baz.bar, baz.baz)
})

This causes the decoration logic to run three times, once per class.

With legacy decorators (and [[Set]] semantics), that looks like this:

class Foo {
  @observable foo = 123
}

class Bar extends Foo {
  @observable bar = 456
}

class Baz extends Bar {
  @observable baz = 'hello'
}

const baz = new Baz

autorun(() => {
  console.log(baz.foo, baz.bar, baz.baz)
})

Because of the new [[Define]] semantics breaking things, here's what the new MobX 6 will look like (discusson issue):

class Foo {
  foo = 123

  constructor() {
    makeObservable(this, { foo: observable })
  }
}

class Bar extends Foo {
  bar = 456

  constructor() {
    super()
    makeObservable(this, { bar: observable })
  }
}

class Baz extends Bar {
  baz = 'hello'

  constructor() {
    super()
    makeObservable(this, { baz: observable })
  }
}

const baz = new Baz

autorun(() => {
  console.log(baz.foo, baz.bar, baz.baz)
})

where makeObservable must now be called on every instance of the classes, in every constructor so that it can define the observable property descriptors after the class fields [[Define]] happens.

For one instance of Baz, the makeObservable function has to be called three times! And for each call, it performs a (possibly short-circuited) prototype iteration.

This is obviously considerably more expensive than if we simply had [[Set]] semantics and could therefore simply apply the features at class-definition time.

Here's the same as the previous example, but using decorators (legacy or newer ones):

class Foo {
  @observable foo = 123

  constructor() {
    makeObservable(this)
  }
}

class Bar extends Foo {
  @observable bar = 456

  constructor() {
    super()
    makeObservable(this)
  }
}

class Baz extends Bar {
  @observable baz = 'hello'

  constructor() {
    super()
    makeObservable(this)
  }
}

const baz = new Baz

autorun(() => {
  console.log(baz.foo, baz.bar, baz.baz)
})

It is less verbose and more DRY, but still suffers from runtime overhead beyond class-definition time.

In the old system, 1000 objects of type Baz meant only 3 calls fo decorate. But now, 1000 Baz objects means 3000 calls to makeObservable.

Who knows when the new decorators will come out (which would allow fixes), but the current landscape is in bad shape now, thanks to [[Define]] semantics.

littledan commented 4 years ago

I'm optimistic that the decorators proposal will resolve these concerns.

trusktr commented 4 years ago

How many years from now? EDIT: It's more of a statement than a question: of course it is hard to tell, but for some significant amount of time there will be turmoil in the code people write. The existence of MobX's version 6 bump is literally caused by this proposal. It is not ideal to force well-known libraries to undergo such changes.