Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comparison between libui-node & libui-napi #11

Open
parro-it opened this issue May 4, 2018 · 41 comments
Open

Comparison between libui-node & libui-napi #11

parro-it opened this issue May 4, 2018 · 41 comments

Comments

@parro-it
Copy link
Owner

parro-it commented May 4, 2018

I completed the initial implementation of windows, boxes and multiline entry.
I then created a new branch from 0_3_0 branch of libui-node, removed all the stuff except windows, boxes and multiline entry.
Here is a comparison of the two implementation to help decide if the N-API rewrite is worth the effort:

nbind version

binary size: 627Kb
compilation time: 39s
runtime memory consuption of example.js: 21M

N-API version

binary size 56Kb
compilation time: 7s
runtime memory consuption of example.js: 18M

Considering also that N-API greatly simplify publish of prebuilt binaries, and that it offer much control on memory management and type conversion, my opinion is that the conversion it's totally
worth the effort.

@mischnic
Copy link
Contributor

mischnic commented May 4, 2018

What about compatibility?

@parro-it
Copy link
Owner Author

parro-it commented May 4, 2018

@mischnic
Copy link
Contributor

mischnic commented May 4, 2018

Great, but what I really meant is node support for NAPI.

@parro-it
Copy link
Owner Author

parro-it commented May 4, 2018

It works with node >=8

@parro-it
Copy link
Owner Author

parro-it commented May 4, 2018

Are you concerned on getting support for older versions?

@mischnic
Copy link
Contributor

mischnic commented May 4, 2018

Are you concerned on getting support for older versions?

(Most likely not worth it.)

@mischnic
Copy link
Contributor

mischnic commented May 4, 2018

It works with node >=8

libui-napi/package.json

Lines 19 to 21 in ee33206

"engines": {
"node": ">=10"
},

Well..

@parro-it
Copy link
Owner Author

parro-it commented May 4, 2018

Fixed!

@parro-it
Copy link
Owner Author

parro-it commented May 4, 2018

Regarding how to organize the development, I'm in doubt if it would be better to move all the stuff
I already did here to a new branch in libui-node, or otherwise keep libui-napi as a separate repo and
use libui-node only for the wrapper classes.
Any thought?

@mischnic
Copy link
Contributor

mischnic commented May 4, 2018

use libui-node only for the wrapper classes.

What wrapper classes?

@parro-it
Copy link
Owner Author

parro-it commented May 4, 2018

These: https://github.com/parro-it/libui-napi/tree/master/js
The native part now bind libui functions one-by-one, and these classes should provide the same API as previews release.

@mischnic
Copy link
Contributor

mischnic commented May 4, 2018

Any thought?

Not sure.
Everything being in one place might make development easier.

@mischnic
Copy link
Contributor

mischnic commented May 4, 2018

What about linux and windows?

Windows doesn't like the _Atomic(bool) syntax.
Linux doesn't know pthread_rwlock_t.

@parro-it
Copy link
Owner Author

parro-it commented May 4, 2018

Everything being in one place might make development easier.

Yes, I agree.

@parro-it
Copy link
Owner Author

parro-it commented May 7, 2018

Windows doesn't like the _Atomic(bool) syntax.

By now, I removed the _Atomic altogether... I should think of a better solution...

Linux doesn't know pthread_rwlock_t.

Are we using it?

@mischnic
Copy link
Contributor

mischnic commented May 7, 2018

Are we using it?

No, it's used in uv.h and was fixed by changing -std=c11 to -std=gnu11.

@parro-it
Copy link
Owner Author

parro-it commented May 7, 2018

No, it's used in uv.h and was fixed by changing -std=c11 to -std=gnu11.

Now I get the reason for that change...

@parro-it
Copy link
Owner Author

parro-it commented May 7, 2018

The Atomic usage could probably be replaced by some other thread synchronization function implemented by libuv

@mischnic
Copy link
Contributor

mischnic commented May 7, 2018

Now I get the reason for that change...

Took quite a while to find that on the internet.

@parro-it
Copy link
Owner Author

parro-it commented May 7, 2018

Took quite a while to find that on the internet.

I guess, there is no much documentation or example around...

I have a collection of bookmark on some reference related to node event loop and libuv docs / example / article. How could I share them?

@mischnic
Copy link
Contributor

mischnic commented May 7, 2018

a gist?

@parro-it
Copy link
Owner Author

parro-it commented May 7, 2018

https://gist.github.com/parro-it/842ff46a86142c30b1f94f97bb738418
The titles are not saved unfortunatly... ☹️

@mischnic
Copy link
Contributor

mischnic commented May 8, 2018

Are you concerned on getting support for older versions?

N-API is now available in Node 6.14.2. If we want to, these changes would be required:

  • use the polyfill for async-hooks (the official creditkarma/async-hooks - mine isn't needed).
  • async and await aren't supported 😞 (in the download scripts).

@parro-it
Copy link
Owner Author

parro-it commented May 8, 2018

N-API is now available in Node 6.14.2.

Awesome! Is that a new release where they backported N-API or what?

async and await aren't supported 😞 (in the download scripts).

We can change that stuff, not a great deal...

(the official creditkarma/async-hooks - mine isn't needed)

If I remember correctly, using your we can support node 4?

@parro-it
Copy link
Owner Author

parro-it commented May 8, 2018

If I remember correctly, using your we can support node 4?

Ah no, because it doesn't have N-API I guess...

@mischnic
Copy link
Contributor

mischnic commented May 8, 2018

Awesome! Is that a new release where they backported N-API or what?

Exactly.

If I remember correctly, using your we can support node 4?

Ah no, because it doesn't have N-API I guess...

Exactly. 😄

@mischnic
Copy link
Contributor

mischnic commented May 8, 2018

Why is the library file called node_libui.node while this will be called "libui-node"?

@parro-it
Copy link
Owner Author

parro-it commented May 8, 2018

Why is the library file called node_libui.node while this will be called "libui-node"?

Uhm, no reason...

@parro-it
Copy link
Owner Author

After 21 days of work, the port is almost complete, so I thought to repeat the comparison with the full version of libui-node@0.2.0

nbind version
binary size: 627Kb
compilation time: 39s
runtime memory consuption of example.js: 21M

N-API version
binary size 3,0M
compilation time: 234s
runtime memory consuption of example.js: 19M

prebuilt binaries are now available for linux, windows & macOS 32bit.
Maybe we should support also for 32bit versions?

Also the structure of the repo (folders, files, tools) should be almost stable, so I think we could merge back this repo into a new branch 0_4_0 on libui-node as soon are tester branch is merged

@mischnic
Copy link
Contributor

Maybe we should support also for 32bit versions?

Not needed for macOS. How many 32bit Windows and Linux users are there?

@mischnic
Copy link
Contributor

So what's your plan? If this becomes libui-node 0.4.0, what changes would be in 0.3.0?

@parro-it
Copy link
Owner Author

So what's your plan? If this becomes libui-node 0.4.0, what changes would be in 0.3.0?

I thought two option here:

  • Just keep in 0.3 what we already started to work on (the two existing PRs). Move everything else in 0.4.0

  • Keep in 0.3.0 everything not already solved/implemented in libui-napi

I honestly prefer the first option, I fear the second will force us to implements some pieces two times...

@mischnic
Copy link
Contributor

mischnic commented Nov 2, 2018

Do you want to include tables in the next libui-node (=napi) release?

@parro-it
Copy link
Owner Author

parro-it commented Nov 2, 2018

Do you want to include tables in the next libui-node (=napi) release?

Yes, that's the idea. We are not too far away anyway...

@bigopon
Copy link
Contributor

bigopon commented Nov 28, 2018

@parro-it @mischnic From this conversation, I understand that if I want to experiment with libui node, I should start on libui napi? If so, is it in experiment ready state?

@parro-it
Copy link
Owner Author

Well, the project is almost completed, I'm finishing binding of uiTables. So yes, I guess it is in an "experiment ready state".

But what are you thinking to experiment exactly? Are you trying to write a small app?

@parro-it
Copy link
Owner Author

The version published on npm may be not up-to-date.
And be aware that the package name will change when we'll merge back in main libui-node repo

@bigopon
Copy link
Contributor

bigopon commented Nov 28, 2018

@parro-it thanks for prompt reply.
I want to experiment integrating libui node with Aurelia version next (vNext), ideally, it should be along these lines:

// app.ts
@customElement({
  name: 'app',
  template: `<template>
    <ui-form>
      <ui-entry text.bind="firstName" />
      <ui-entry text.bind="middleName" />
      <ui-entry text.bind="lastName" />
      <ui-multiline text.bind="bio" height="50" />
    </form>
  </template>`
})
export class App {
  firstName = 'Andrea';
  middleName = '';
  lastName = 'Parodi';
  bio = '';
}

Also able to define your own custom element.

This is to verify our renderer of version next, which has worked well so far in my experiment with PIXIjs, NativeScript, and server side rendering. if you are interested, here is the repo

About the experiment, thanks for the heads up, I'll give it a go (just finished installing 0.2 though)

@parro-it
Copy link
Owner Author

Oh, nice experiment... I remember I spoke with @EisenbergEffect months ago about an Aurelia renderer, but can't recall the issue ATM...

About the experiment, thanks for the heads up, I'll give it a go (just finished installing 0.2 though)

Anyway, libui-napi will have the same public API of previuos libui-node (plus table API that we won't implement in old versions)

@bigopon
Copy link
Contributor

bigopon commented Nov 28, 2018

Just installed, beside 2 warnings, I guess everything is good. Will update with result. Thanks.

@EisenbergEffect
Copy link

@parro-it It was a while ago that we talked but we're finally at a place where we can start working with what you've built. We're deep into our vNext implementation for Aurelia and we're proving out our renderer design to make sure we can render to various interesting targets besides just HTML. @bigopon is spearheading that effort and libui is next on the list :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants