packer-community / winrmcp

Copy files to a remote host using WinRM
MIT License
58 stars 31 forks source link

Decode memory-efficiently copied files from base64 #10

Closed dusek closed 7 years ago

dusek commented 8 years ago

Current code decodes the base64-encoded representation of the copied file by 3 steps during which it holds the whole file in memory:

So let's change it into being more efficient - by performing this decoding pipeline line-by-line: read one line of base64 file, decode it into bytes, and write (append) it to the destination binary file, and repeat until whole file is converted.

I tested correctness by computing md5 of the binary result and comparing with md5 sum computed on the machine on which winrmcp was run, in these scenarios:

Performance-wise: on a Win7 Pro x64 machine without the VirtualBox additions installed, copying a 187MB file took 11 minutes for copying over the temporary base64 representation, and approx. 12-13 seconds for decoding the base64 representation into original binary form. I have not compared it the current method, nor tried on a machine with VirtualBox additions installed.

This prevents OutOfMemoryException in some cases (most notably when packer copies VBoxGuestAdditions.iso during some installations of Windows).

Fixes #9

mefellows commented 8 years ago

Thanks @dusek, this is great. I'll take a look through and provide some feedback.

EikeVerdenhalven commented 7 years ago

The loop can be written a little more compact. See #19 (which was my attempt, before I saw it's a dupe).

dylanmei commented 7 years ago

@dusek Would you kindly resolve any conflicts? If it's been too long we'll work with @EikeVerdenhalven -- the changes you have both made are similar.

dusek commented 7 years ago

@dylanmei rebased (the only conflict seems to have been due to removing trailing whitespace in fd76b28 at winrmcp/cp.go:130

dylanmei commented 7 years ago

Fantastic, thanks!