shadowsocks / shadowsocks-chromeapp

Chrome client for shadowsocks
696 stars 541 forks source link

Migrate to ECMAScript 6 #21

Open librehat opened 8 years ago

librehat commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

librehat commented 8 years ago

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