Reduced memory usage by reading files using extractBufferedDataFromFile…#28
Reduced memory usage by reading files using extractBufferedDataFromFile…#28abbeycode merged 5 commits intoabbeycode:masterfrom
Conversation
…le instead of readFile. Addresses abbeycode#27 Signed-off-by: Brendan Duddridge <brendand@gmail.com>
There was a problem hiding this comment.
I'm curious what the reason is for this change. Is it a multithreading concern?
There was a problem hiding this comment.
Yes, I think I read somewhere that it was best to create your own NSFileManager if you were doing anything in the background.
|
I had one question I commented inline, but also these general concerns:
Take a look at the above, and then I'll take a closer look at the rest of your changes. |
|
Sorry, I guess I have tabs set instead of spaces in Xcode. Hopefully if I change that it won't screw up my own version control. I think it was something to do with the password not being provided. I didn't change any of that code and it looked like all other tests passed. |
Wasn't passing up the proper error code when unzipping a password protected archive without providing a password. Also wasn't deleting the directory we created if the unarchiving failed. All tests pass now. Also had to add "import UnzipKit" to WriteDataTests.swift. Failed to compile otherwise. Signed-off-by: Brendan Duddridge <brendand@gmail.com>
|
Switching your Xcode preference will affect new edits you make, but for now can you please do a find and replace (find |
There was a problem hiding this comment.
Please fix this typo: defalatedDirectoryURL instead of deflatedDirectoryURL
|
I've added some inline comments, but this is looking good. Thanks again! |
Signed-off-by: Brendan Duddridge <brendand@gmail.com>
|
I've left a couple more inline comments. Thanks for sticking with it. We're close; I can feel it! I can replace the tabs with spaces when I merge, if you'd prefer to change your Xcode preference back. |
|
Oops. I did do a search and replace and have set Xcode to use spaces. Not sure why it didn't change though. I'll get to these issues later as I have to go out to demo the next version of my app to an iOS dev meetup. I'll get rid of that dataLength var if it's no longer needed and add the file handle check. |
Signed-off-by: Brendan Duddridge <brendand@gmail.com>
|
Do you think this is suitable to merge into master now? |
|
|
||
|
|
||
| if (!deflatedFileHandle) { | ||
| [self assignError:&strongError code:UZKErrorCodeFileHandleCreate |
There was a problem hiding this comment.
I don't think it's necessary to create a new error code for this case. Just use the UZKErrorCodeOutputError code used elsewhere in the method.
|
Hey, sorry, I missed the commit you pushed. I took a look and made an inline comment. Take care of that, then I'll fix the tab-to-space conversion and merge manually. |
…deOutputError instead. Signed-off-by: Brendan Duddridge <brendand@gmail.com>
|
ok, fixed. Hopefully it's ok now. |
|
All set now. Thanks for suggesting and writing this. Awesome job! |
2 participants
… instead of readFile. Addresses #27
Files were previously read whole into memory and then written out. This change streams them instead, reducing the amount of memory required to unzip files.