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

Migrate to ECMAScript 6 #21

Open
librehat opened this issue Mar 12, 2016 · 5 comments
Open

Migrate to ECMAScript 6 #21

librehat opened this issue Mar 12, 2016 · 5 comments

Comments

@librehat
Copy link
Contributor

Migrate from CoffeeScript to standard ECMAScript 6. A lot of new features available in ES6, on the other hand, it helps JavaScript programmer (but non-CofeeScript programmer) to contribute.

If it seems okay, I can submit a pull request to keep JavaScript(/lib) code but remove CoffeeScript (/src). The second stage, I propose, is to use asynchronous promise to gain speed boosts.

@nekolab
Copy link
Collaborator

nekolab commented Mar 13, 2016

I think that's a good idea for migrate to es6, I'm planning to do it after nacl version is stable enough (actually I want to rewrite the whole app after nacl is stable).

On the other hand, use Promise instead of callback seems not a good idea, although the promise is more clearly than callback hell and has a native implementation in modern browser, it still slower than callback, especially when the chrome socket api is using the callback and you need convert it to promise manually.

@librehat
Copy link
Contributor Author

The speed doesn't differ much (tested on Windows 10 x64/Opera 36 (Chromium 48)), if promise is not faster, https://jsperf.com/promise-vs-callback (I do find promise is much slower in Microsoft Edge)

I would suggest keep shadowsocks-nacl as a separate project, while keep this one as a pure JS implementation of shadowsocks.

@nekolab
Copy link
Collaborator

nekolab commented Mar 13, 2016

Well. I still believe introduce promise into shadowsocks is not a good idea.

I think promise should be used in the place which suffered from callback hell (current we have no callback hell), have complex logic need to clarify, or not performance-sensitive.

Since window.Promise is an object, new a Promise will cost much than directly call a callback. You may noticed the JS implementation will eat a great number of memory when downloading big files, I believe it's caused by GC cannot recycle the used Uint8Array and forge binary string object when engine is still busy, the situation will become worse if we create more Promise object in it.

In short: I don't think the logic in shadowsocks is too complex or we had fell into callback hell, and since the network application is too damn performance-sensitive, there is no need to add something may / also may not cause performance issue into our code.

As I noticed above, the JS implementation has it's congenital defects which cannot be fixed in JS code level, not only about the GC, but also includes the single thread, crypto performance and some others, use the native client technology can fix these to a certain extent. The JS implementation will not be deprecated, instead it will be treated as the fallback when native client module is not available. Provides two types of chrome apps will confused people, makes them hard to choice, and since native client module could be crashed / lack of interpreter in chrome, the JS fallback must be existed, so I want to put these two implementation together into this project, the NaCl project will only provides pure NaCl module for any other web developers who wants to use it in their web apps.

Feel free to let me know if you have any questions or advice, thanks :)

@nekolab
Copy link
Collaborator

nekolab commented Mar 13, 2016

PS: The NaCl module will be imported in binary form, I will not include any C++ code into this project

@librehat
Copy link
Contributor Author

Cool, that's a good solution, JS as a fallback backend.

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

2 participants