imba benchmark

[krausest]

Sorry - I tried to merge, but ran into an issue: For some reason I don't understand the buttons don't get id attributes which means the test driver can't run the benchmark:

Can you please take a look at it?

[leeoniya]

maybe this will work?

<button.btn.btn-primary.btn-block :type='button' :id=@id

append #@id:

<button.btn.btn-primary.btn-block#@id :type='button' :id=@id

[krausest]

Nice try, but...

ERROR in ./src/main.imba
Module Error (from ./node_modules/imba/loader.js):
Parse error at [969:3]: Unexpected 'IDENTIFIER'
      30 |              <self>
      31 |                      <div.col-sm-6.smallpad>
   -> 32 |                              <button.btn.btn-primary.btn-block#@id :type='button' :id=@id :tap=@cb> @title

(stays the same when removing the :id or pushing @id to the front)

[allain]

I'll take a stab shortly.

On Tue, Dec 4, 2018, 3:34 PM Stefan Krause <notifications@github.com wrote:

Nice try, but...

ERROR in ./src/main.imba Module Error (from ./node_modules/imba/loader.js): Parse error at [969:3]: Unexpected 'IDENTIFIER' 30 | 31 | -> 32 | <button.btn.btn-primary.btn-block#@id :type='button' :id=@id :tap=@cb> @title

(stays the same when removing the :id or pushing @id https://github.com/id to the front)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/krausest/js-framework-benchmark/pull/492#issuecomment-444248124, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG4b4bQUCDIgvd-F4MzkdwIuZyW10tpks5u1txwgaJpZM4YwhXl .

[allain]

Changing :id=@id to id=@id worked the dom element now has the correct id.

[allain]

I'm not sure if this particular test belongs in keyed or unkeyed to be honest.

[krausest]

Thanks - I‘ll take a look at it tomorrow.

[leeoniya]

I'm not sure if this particular test belongs in keyed or unkeyed to be honest.

npm run isKeyed -- --framework imba should tell you

[allain]

I mean I know wrote it in the keyed directory but I'm not sure if the code satisfies the definition of a keyed framework test.

On Tue, Dec 4, 2018, 4:55 PM Leon Sorokin <notifications@github.com wrote:

I'm not sure if this particular test belongs in keyed or unkeyed to be honest.

npm run isKeyed -- --framework svelte should tell you

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/krausest/js-framework-benchmark/pull/492#issuecomment-444273875, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG4b1Qno5sETlduDURNxuJ_sAebWH_Xks5u1u9jgaJpZM4YwhXl .

[localvoid]

@allain I'm not sure if the code satisfies the definition of a keyed framework test.

I think that keys are assigned with @{key} syntax: https://github.com/somebee/dom-reconciler-bench/blob/29041f8cd28a4e38f9a830ad8ab3cc2adbe44550/apps/imba/app.imba#L86

[allain]

@localvoid You are absolutely correct, I've updated the Rows to be rendered with a key according to the syntax you pointed to.

Thank you.

I'll copy that one over to the unkeyed directory and remove the key syntax too.

[localvoid]

@allain

$ cd webdriver-ts
$ npm run isKeyed -- --framework imba-v1.4.1-keyed

> webdriver-ts@1.0.0 isKeyed /home/void/w/js/garbage/js-framework-benchmark/webdriver-ts
> cross-env LANG="en_US.UTF-8" node dist/isKeyed.js "--framework" "imba-v1.4.1-keyed"

Frameworks that will be checked imba-v1.4.1-keyed
imba-v1.4.1-keyed is non-keyed for 'run benchmark' and non-keyed for 'remove row benchmark' non-keyed for 'swap rows benchmark' . It'll appear as non-keyed in the results
ERROR: Framework imba-v1.4.1-keyed is not correctly categorized

Key should be assigned on the <Row>:

<Row@{item:id} item=item selected=(selected === item:id) select=(do select(item)) remove=(do remove(item))> 

$ npm run isKeyed -- --framework imba-v1.4.1-keyed

> webdriver-ts@1.0.0 isKeyed /home/void/w/js/garbage/js-framework-benchmark/webdriver-ts
> cross-env LANG="en_US.UTF-8" node dist/isKeyed.js "--framework" "imba-v1.4.1-keyed"

Frameworks that will be checked imba-v1.4.1-keyed
imba-v1.4.1-keyed is keyed for 'run benchmark' and keyed for 'remove row benchmark' keyed for 'swap rows benchmark' . It'll appear as keyed in the results

[krausest]

@allain I can't merge it in the current state. Can you please update it to generate the correct html and the class for selected rows as @localvoid reviewed? Since the non-keyed version works (after changing the class for selected rows) I took a first look at the performance (though it's not really comparable until the identical HTML is generated):

[allain]

I've implemented some of the fixes, but the aria-* attributes required me to submit a pull request to the imba project. once it's merged I'll finish and push.

• Allain

On Wed, Dec 5, 2018 at 2:59 PM Stefan Krause notifications@github.com wrote:

@allain https://github.com/allain I can't merge it in the current state. Can you please update it to generate the correct html and the class for selected rows as @localvoid https://github.com/localvoid reviewed? Since the non-keyed version works (after changing the class for selected rows) I took a first look at the performance (though it's not really comparable until the identical HTML is generated): [image: bildschirmfoto 2018-12-05 um 20 52 56] https://user-images.githubusercontent.com/3374055/49540346-30c67a80-f8d0-11e8-96fa-efc05002bab0.png

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/krausest/js-framework-benchmark/pull/492#issuecomment-444624559, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG4b9ALnDVUYRwULVIMJn7JUv3ewBgOks5u2CWPgaJpZM4YwhXl .

[localvoid]

As a temporary solution it is possible to assign aria-hidden this way:

tag GlyphIcon < span
    def build
        set('aria-hidden', 'true')

    def render
        <self.glyphicon.glyphicon-remove>

[allain]

Thanks. I'll go ahead and apply that change.

On Thu, Dec 6, 2018 at 4:17 AM Boris Kaul notifications@github.com wrote:

As a temporary solution it is possible to assign aria-hidden this way:

tag GlyphIcon < span def build set('aria-hidden', true)

def render

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub , or mute the thread .

[allain]

Done. Thank you Boris

[krausest]

@allain Keyed looks perfect, thanks! The non-keyed version is identical, isn't it? (At least it's keyed too according to my test) I could merge the PR with just the keyed version if that's ok for you.

[allain]

I thought I'd removed the @{item:id} from the non-keyed one.

I'll have a look later.

On Thu, Dec 6, 2018 at 4:16 PM Stefan Krause notifications@github.com wrote:

@allain https://github.com/allain Keyed looks perfect, thanks! The non-keyed version is identical, isn't it? (At least it's keyed too according to my test) I could merge the PR with just the keyed version if that's ok for you.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/krausest/js-framework-benchmark/pull/492#issuecomment-445032399, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG4b5HmqPzrVy-1edAKtjlF_gstt9yRks5u2YlJgaJpZM4YwhXl .

[krausest]

Non-keyed is fine after I removed it. I hope to post results tomorrow.

[allain]

Woohoo. Thanks!

On Thu, Dec 6, 2018 at 5:02 PM Stefan Krause notifications@github.com wrote:

Merged #492 https://github.com/krausest/js-framework-benchmark/pull/492 into master.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/krausest/js-framework-benchmark/pull/492#event-2011075165, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG4b_LUGDLCVkzw06AA89JYzEudtrG-ks5u2ZPmgaJpZM4YwhXl .

[PierBover]

@krausest please let us know when we can see the results!

[krausest]

@allain @PierBover First results - please note that only imba ran on chrome 71, all other benchmarks were measured on chrome 70. Hopefully I can rerun all benchmarks this weekend.

[leeoniya]

when playing with imba's reconciler benchmarks, i noticed that imba has a performance cliff at 500 siblings, probably due to [1]

[1] https://github.com/imba/imba/blob/5f7586705a40d40cd9cd8a5206f69629a4bfff61/src/imba/dom/reconciler.imba#L201

[PierBover]

Maybe @somebee can comment on that

[krausest]

@allain @PierBover The benchmark has been updated for Chrome 71:

https://krausest.github.io/js-framework-benchmark/current.html