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

Improving transfer speed #42

Closed
T-vK opened this issue Jun 14, 2017 · 5 comments
Closed

Improving transfer speed #42

T-vK opened this issue Jun 14, 2017 · 5 comments

Comments

@T-vK
Copy link
Contributor

T-vK commented Jun 14, 2017

What can we do to increase the transfer speed? I tried using baudrates of up to 460800, but it doesn't really seem to make a big difference compared to 115200. I mean it might be a little bit faster, but definitely not 4x as fast, at least for me. So I started digging in the source code a little and found some things that could (potentially?) be improved.

  • The regex that produces the chunks. This regex /.{1,232}/g creates a new chunk for every single line (unless it is longer than 232 chars then it splits the same line into even more chunks).
    Is there a good reason to do this? Why don't we just use /.{1,232}/gs instead so that we match over multiple lines, making every chunk as big as possible? Are new line characters a problem or can we just escape them? And btw is there a reason for why you choose 232 in the first place?
  • __nmtwrite. Every time we send a chunk, we introduce overhead because of the function name of __nmtwrite. Wouldn't it make sense to choose something shorter? Maybe a dynamic name that is chosen based on what is available. For instance if the variable name _ is not in use yet, we could simply use that.
    If you are transferring files that have 5000 lines of code in them then you would really save a lot of bandwidth. I mean for 5000 chunks you would introduce 5000*12=60000bytes of overhead. if the function name was only one char _() then you'd only have 5000*3=15000bytes of overhead.

Any ideas what else could be done? Maybe a lightweight compressing/decompressing mechanism?

@AndiDittrich
Copy link
Owner

Dear T-vK,

thanks for your recommendations.

  1. Of course, the regex is a bit weak...its better to replace it with native Nodejs Buffer to extract the chunks. i did not know that a s modifier was available in javascript yet ?!

  2. The name was chosen to avoid any kind of namespace collisions. i won't like to change this.

  3. Do you use the native base64 encoding added to one of the last releases ? it is much faster then the old hex encoding

  4. As a more elegant solution NodeMCU should use its own transfer protocol based on tcp/uart directly without invoking the lua cli interpreter (that is the real slow part).

br, Andi

AndiDittrich added a commit that referenced this issue Jun 14, 2017
…t match linebreaks (slow transfer speed because of short chunks) #42
@AndiDittrich
Copy link
Owner

the issue with the chunk size should be fixed with the following regex:

var chunks = content.match(/[\s\S]{1,232}/g) || [];

@T-vK
Copy link
Contributor Author

T-vK commented Jun 14, 2017

The name was chosen to avoid any kind of namespace collisions. i won't like to change this.

Well, my idea was:

if a == nil then
    a = function() .....
else
    -- the variable does already exist
    -- start from the beginning with a different variable name
end

-- transfer the file

a = nil

I think it's been a while since I have updated nodemcu-tool, so I might not have the base64 encoding.

Changing the UART protocol to not invoke the cli is probably not possible, right?
Hmm, flashing over TCP would actually be pretty cool, though. I could finally flash my ESPs from Android then...

Oh and I was wrong about the s flag. It's not part of JS yet. It might make it into ES 2018 though, seeing that it's in stage 3. :)

@AndiDittrich
Copy link
Owner

i know your idea..but it causes to much overhead within the transfer helper function and will currently break the nodemcu tool.

please take a look at the latest NodeMCU tool version including base64 encoding (requires the encode firmware modules]

@AndiDittrich
Copy link
Owner

did you tried base64 based transfer yet ? in case it meet your demands i will close this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants