Skip to content

Conversation

vkurchatkin
Copy link
Contributor

@chrisdickinson
Copy link
Contributor

Non-blocking comment: this seems to increase the number of round trips between JS and C++ for a given Alloc call; does moving the type checking logic to JS balance that out, perf-wise?

@chrisdickinson
Copy link
Contributor

(Otherwise this looks good to me pending @trevnorris' +1)

@Fishrock123
Copy link
Contributor

ping @trevnorris

Copy link
Contributor

Choose a reason for hiding this comment

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

Mind switching these to ASSERT()? Useful for when we're calling these directly from core.

@trevnorris
Copy link
Contributor

couple minor comments. LGTM otherwise.

@vkurchatkin
Copy link
Contributor Author

@trevnorris added asserts. PTAL

@trevnorris
Copy link
Contributor

@vkurchatkin LGTM. Great work.

vkurchatkin added a commit that referenced this pull request Mar 4, 2015
PR-URL: #920
Reviewed-By: Chris Dickinson <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
vkurchatkin added a commit that referenced this pull request Mar 4, 2015
PR-URL: #920
Reviewed-By: Chris Dickinson <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
@vkurchatkin
Copy link
Contributor Author

Thanks! landed in 0697f8b and 8f5f12b

@vkurchatkin vkurchatkin closed this Mar 4, 2015
@rvagg rvagg mentioned this pull request Mar 4, 2015
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

Successfully merging this pull request may close these issues.

4 participants