Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

doc: add summit meeting minutes #5

Closed
wants to merge 1 commit into from

Conversation

joshgav
Copy link
Contributor

@joshgav joshgav commented Mar 6, 2017

These notes are stored here: https://github.com/nodejs/abi-stable-node/tree/doc/meeting%20notes

But thought they might be better off here for reference. @aruneshchandra @mhdawson thoughts?

@joshgav joshgav mentioned this pull request Mar 6, 2017
@aruneshchandra
Copy link

@joshgav - This repo does seem like a better place to collect these notes.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM other than my suggestion of wording change around canvas.


Canvas perf regression is likely because there’s heavy usage of V8 APIs so much
more marshaling needed. Also first tried with C++ wrapper. No optimizations yet
either.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest

Canvas perf regression is likely because the performance benchmark makes calls to canvas where the work done in the native method is very small. This means that any overhead, even the limited overhead added by N-API has a significant impact.

joshgav added a commit that referenced this pull request Mar 24, 2017
PR-URL: #5
Reviewed-By: Arunesh Chandra <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@joshgav
Copy link
Contributor Author

joshgav commented Mar 24, 2017

landed in e86328b

@joshgav joshgav closed this Mar 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants