pdfbox-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tilman Hausherr (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (PDFBOX-4260) Reduce RAM requirement of COSOutputStream
Date Sat, 07 Jul 2018 18:46:00 GMT

    [ https://issues.apache.org/jira/browse/PDFBOX-4260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16535860#comment-16535860
] 

Tilman Hausherr edited comment on PDFBOX-4260 at 7/7/18 6:45 PM:
-----------------------------------------------------------------

Thank you for your patch! However… TestPDPageContentStream fails when building the trunk: 

{noformat}
testSetCmykColors(org.apache.pdfbox.pdmodel.TestPDPageContentStream)  Time elapsed: 0.233
s  <<< ERROR!
 java.io.IOException: Buffer already closed
  at org.apache.pdfbox.pdmodel.TestPDPageContentStream.testSetCmykColors(TestPDPageContentStream.java:44){noformat}
 
This is because in that test, the content stream is closed twice. This was missed in refactoring
and I'll delete that line later and add a real test instead, but it shows that you're not
respecting the {{Closeable.close()}} contract.

An improvement/simplifcation wish: you can also make the assumption that {{scratchfile}}
constructor parameter is never null (the only usage is in {{COSStream.createOutputStream()}}
and it is never null there), this would simplify (y)our code, please correct the javadoc when
resubmitting. {{COSOutputStream}} is package-local so it can be changed without breaking
the public API.


was (Author: tilman):
Thank you for your patch! However… TestPDPageContentStream fails: 

{noformat}
testSetCmykColors(org.apache.pdfbox.pdmodel.TestPDPageContentStream)  Time elapsed: 0.233
s  <<< ERROR!
 java.io.IOException: Buffer already closed
  at org.apache.pdfbox.pdmodel.TestPDPageContentStream.testSetCmykColors(TestPDPageContentStream.java:44){noformat}
 
This is because in that test, the content stream is closed twice. This was missed in refactoring
and I'll delete that line later and add a real test instead, but it shows that you're not
respecting the {{Closeable.close()}} contract.

An improvement/simplifcation wish: you can also make the assumption that {{scratchfile}}
constructor parameter is never null (the only usage is in {{COSStream.createOutputStream()}}
and it is never null there), this would simplify (y)our code, please correct the javadoc when
resubmitting. {{COSOutputStream}} is package-local so it can be changed without breaking
the public API.

> Reduce RAM requirement of COSOutputStream
> -----------------------------------------
>
>                 Key: PDFBOX-4260
>                 URL: https://issues.apache.org/jira/browse/PDFBOX-4260
>             Project: PDFBox
>          Issue Type: Improvement
>          Components: Writing
>    Affects Versions: 2.0.11
>            Reporter: Jesse Long
>            Priority: Minor
>             Fix For: 2.0.12, 3.0.0 PDFBox
>
>         Attachments: pdfbox-cosstream.patch
>
>
> COSOutputStream uses a byte array to buffer data prior to filtering. This is bad. As
commented in COSOutputStream, it should be updated to use a scratch file buffer instead.
> This patch does that.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org


Mime
View raw message