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

proposal; never throw #10

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

proposal; never throw #10

wants to merge 2 commits into from

Conversation

sonnyp
Copy link
Owner

@sonnyp sonnyp commented Apr 18, 2016

let the consumer throws but catch so that a polygoat function never throws and calls the callback with the exception as first argument instead

related discussion #9

maybe a new module? (with dezalgoing as well)

@sonnyp sonnyp mentioned this pull request Apr 18, 2016
@sonnyp sonnyp changed the title proposal, disallow throw proposal; never throw Apr 18, 2016
@benjamingr
Copy link

in any case I guess it's okay to assume the performance impact is null.

Seriously, why would you do that? You acknowledge that the benchmark doesn't make sense because you introduced a try/catch which is a deopt in v8 and instead of viewing some presentations about it you just assume there is no impact?

@sonnyp
Copy link
Owner Author

sonnyp commented Apr 18, 2016

Seriously, why would you do that? You acknowledge that the benchmark doesn't make sense because you introduced a try/catch which is a deopt in v8 and instead of viewing some presentations about it you just assume there is no impact?

What I intended to express is that I (as a user and for my current use case and at the moment) didn't really care for the performance impact. Same thing with eval optimisation. (updated my comment on the benchmark to reflect that EDIT: actually removed the section about benchmark)

@benjamingr
Copy link

That's entirely fine, but

a) As a library author I think you should validate that since it's not just your code that's at stake.
b) You probably shouldn't post benchmarks if you know they are wrong.

Benchmarking v8 is a lot of fun, I usually don't attempt it myself except for when I absolutely have to. If you want to be serious about performance you have to do that and if you don't that's also entirely fine (most Node libs aren't really performance sensitive)

@sonnyp
Copy link
Owner Author

sonnyp commented Apr 18, 2016

Let me re-iterate, at this moment I don't care much for polygoat performances although if anyone is and wish to contribute to that end I'm all for it.

I'm interested in your opinion on the content of this PR though.

@benjamingr
Copy link

I'm -1 on it, I think this is not what callback users expect.

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.

2 participants