Hi DM,<div><br></div><div>If the block is big then I would need to look at changing the BlockType of the bible to chapter from book but I am hoping to avoid that because it would be nice just to download standard documents form CrossWire and all the bibles I have seen have a block type of book. Although if there were some bibles with a block type of chapter I think it would be more suitable for a mobile.</div>
<div><br></div><div>Setting the expectedLength on the baos can save up to 2x the amount of memory allocation e.g. if required size is1025 then allocated memory will reach 2048 using the default doubling method before being sufficient.</div>
<div><br></div><div>I haven't gone into InflaterInputStream and couldn't follow all your logic but are you agreeing that removing the redundant buffer and setting the initial size of the baos correctly are good ideas? Anyway I found that making both changes had an instant impact on OOMs although it could just be that just one of the changes was significant.</div>
<div><br></div><div>Many thanks</div><div>Martin</div><div><br><div class="gmail_quote">On 26 June 2010 13:21, DM Smith <span dir="ltr"><<a href="mailto:dmsmith@crosswire.org">dmsmith@crosswire.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div style="word-wrap:break-word">Welcome Martin,<div><br></div><div>I'm excited about your work.<br><div><br></div>
<div>Yes you can make changes! But not directly. Only a few people can commit changes directly.</div><div><br></div><div>The preferred method is to open an issue in Jira at <a href="http://www.crosswire.org/bugs" target="_blank">www.crosswire.org/bugs</a> under the appropriate bucket. In this case it would be "Common Java Utilities". And attach a patch to it.</div>
<div><br></div><div>This does several things:</div><div>1) It keeps the issue from getting lost in this mailing list.</div><div>2) It allows for discussion/iteration there.</div><div>3) It is a centralized location for changes, allowing one to know what went into a release.</div>
<div>...</div><div><br></div><div>That said, see below:</div><div><br></div><div>In Him,</div><div>DM</div><div><br></div><div><div><div><div class="im"><div>On Jun 26, 2010, at 5:22 AM, Martin Denham wrote:</div><br><blockquote type="cite">
Hi,<div><br></div><div>I am not sure if I can make code changes in the JSword-common but I haven't made any before so I thought I would request the change.</div><div><br></div><div>In Zip.java, in the uncompress method, could you change:</div>
<div><div> ByteArrayOutputStream bos = new ByteArrayOutputStream();</div><div> BufferedOutputStream out = new BufferedOutputStream(bos, expectedLength);</div><div> InflaterInputStream in = new InflaterInputStream(input, new Inflater(), expectedLength);</div>
<div> byte[] buf = new byte[expectedLength];</div></div><div>To:</div><div><div> ByteArrayOutputStream out = new ByteArrayOutputStream(expectedLength);</div><div> InflaterInputStream in = new InflaterInputStream(input, new Inflater(), expectedLength);</div>
<div> byte[] buf = new byte[expectedLength];</div></div><div><br></div><div>There are 2 optimisations:</div><div><ol><li>ByteArrrayOutputStream is passed the expectedLength parameter</li></ol></div></blockquote></div>
I traced through the code and I'm not sure that this fixes the problems. If expectedLength is as is implied by the routine, the expected length of the uncompressed result, and a block is big, there might still be OOMs.</div>
<div><br></div><div>The ByteArrayOutputStream is naive in its allocation of additional space for its internal buffer. If not given a size, it starts at 32 and then doubles each time it needs to grow the buffer. Setting the initial size to the expected size of the final result, eliminates re-allocation. This change is an important speed improvement. And if garbage collection is not optimized, it will save memory.</div>
<div><br></div><div>At the end, the buffer is retrieved with toByteArray, which creates a copy of the buffer. So at a minimum, with this change there are two copies until "bos" is garbage collected. To eliminate that final copy, we'd have to write our own or find something else.</div>
<div><br></div><div>I also looked at InflaterInputStream. Here I am not sure that one should pass it expectedLength. It will set the internal size of its buffer. I'm not sure this buffer needs to be the expected size. It defaults to 512 and once set does not grow. It just controls how often one gets more bytes from the stream to inflate. I'm thinking that perhaps it should be small, either the default or perhaps 1024, 2048, 4096, ... Maybe min(expectedLength, 4096)?</div>
<div><br></div><div>Allocation in Zip.java of buf, probably doesn't need to be the expected size either. It can probably be set to the same as what is passed to the inflater stream.</div><div><br></div><div>So right now, I think that with your improvement, to unzip requires 4x the expected length or more. </div>
<div><br></div><div><br></div><div><div class="im"><br><blockquote type="cite"><div><ol start="2"><li>BufferedOutputStream is not used because there is no benefit to buffering byte array streams.</li></ol></div></blockquote>
</div>Yes, I see this now. Reading the file may need to be buffered, but he byte array streams have their own internal buffering.<br><blockquote type="cite"><div class="im"><div><ol start="2">
</ol><div>These changes may seem trivial but I am attempting to use JSword on an Android mobile and the above prevents many OutOfMemory errors.</div></div><div><br></div><div>Many thanks</div><div>Martin</div></div>
_______________________________________________<br>jsword-devel mailing list<br><a href="mailto:jsword-devel@crosswire.org" target="_blank">jsword-devel@crosswire.org</a><br><a href="http://www.crosswire.org/mailman/listinfo/jsword-devel" target="_blank">http://www.crosswire.org/mailman/listinfo/jsword-devel</a><br>
</blockquote></div><br></div></div></div></div><br>_______________________________________________<br>
jsword-devel mailing list<br>
<a href="mailto:jsword-devel@crosswire.org">jsword-devel@crosswire.org</a><br>
<a href="http://www.crosswire.org/mailman/listinfo/jsword-devel" target="_blank">http://www.crosswire.org/mailman/listinfo/jsword-devel</a><br>
<br></blockquote></div><br></div>