You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pdfbox.apache.org by GitBox <gi...@apache.org> on 2021/04/04 18:23:48 UTC

[GitHub] [pdfbox] valerybokov opened a new pull request #107: potential memory leaks and small performance improvements

valerybokov opened a new pull request #107:
URL: https://github.com/apache/pdfbox/pull/107


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835075692


   COSOutputStream.flush. @THausherr, I think, you need use a super if buffer is null. To revert null check.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-831108088


   ![static_or_local](https://user-images.githubusercontent.com/67366451/116855710-7fdc7b00-ac02-11eb-9a43-a2b81f9d7994.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-855230213


   FontFileFinder.walk. Why additionally not use file.isHidden() for row 123?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-813072890


   > PdfaExtensionHelper.checkNamespaceDeclaration (89). Looks like wrong error type. Should be ErrorType.InvalidPrefix?
   
   That is a good observation, but I don't know enough of that code part, and it also requires a change in a test table, and preflight is slowly getting obsolete anyway because of VeraPDF. I have improved the exception message instead.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1042632955


   A method PDHighlightAppearanceHandler.generateNormalAppearance, line 77. Can rectangle be null? A method 
    PDInkAppearanceHandler.generateNormalAppearance, line 90. The same question.
   
   No checks for several children of PDAbstractAppearanceHandler's.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1042618054


   Hi, @THausherr! You use Graphics type for method PageDrawer.drawPage(Graphics, PDRectangle) as first parameter. But inside you always convert it to Graphics2D. Maybe, you will use Graphics2D in parameter?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-791198178


   ok


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-841353527


   > @valerybokov I tried disabling that part; there are about 22 renderings affected of about 1000 test files with 3000 renderings. Of these, 13 are from test files or documentation files (that explain the 4 affected blend modes). Of the remaining renderings, all the differences are of small size.
   
   As I understood this code uses seldom.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-837047675


   > FDFAnnotationCaret.setSymbol. Maybe it is not a bug but looks weird. 
   
   Yeah, but I prefer not to change it. I don't know what the intention was and it isn't used much.
   
   > BlendComposite.BlendCompositeContext. A field hints is redurant.
   
   thanks fixed
   
   > PatchMeshesShadingContext. The 'patchList' field initializer is redurant.
   
   thanks fixed


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-790879017


   I suspect these tags must be 4 bytes long. CFFParser.readTagName() reads 4 bytes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-788221570


   Thanks, I committed everything except the memory leak. The problem is that I don't know for sure if the object is still used. The tests run, but we don't really have much tests in FDF / XFDF. And the parser was modified to be "on demand" in the trunk, which means that stuff may still be accessed later (however I don't know if this applies to FDF / XFDF, or only for PDF).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-803530477


   ![image](https://user-images.githubusercontent.com/67366451/111898381-58549900-8a2e-11eb-9435-77525ef9e6fc.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-840411665


   > ASCII85InputStream.read(byte[] data, int offset, int len). Row 182. Maybe logical OR.
   
   No, test fails.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-856119473


   > Your code removes all after 0 and including this 0, but that is different to what the current code does. We don't need this at this time.
   
   Your code does the same:
   
   
       public static void main(String[]args) {
           String s=cutNullBytes(new byte[]{ 1,2,3,0,0,0,0,0,5,4,5,4,6,0,0,0},false);
           String s1=cutNullBytes(new byte[]{ 1,2,3,0,0,0,0,0,5,4,5,4,6,0,0,0},true);
           String s2=cutNullBytes(new byte[]{ 1,2,3,5,4,5,4,6,0,0,0,0},false);
           String s3=cutNullBytes(new byte[]{ 1,2,3,5,4,5,4,6,0,0,0,0},false);
           System.out.println(s.equals(s1));
           System.out.println(s2.equals(s3));
       }
   
       private static String cutNullBytes(byte[]bytes, boolean useLastIndexOf) {
           String label = new String(bytes);
           System.out.println("label length: " + label.length() + " label value: " + label);
   
           if (useLastIndexOf) {
               while (label.lastIndexOf(0) != -1)
               {
                   label = label.substring(0, label.length() - 1);
               }
           }
           else {
               int index = label.indexOf(0);
               if (index > -1)
                   label = label.substring(0, index);
           }
   
           System.out.println("label length: " + label.length() + " label value: " + label);
   
           return label;
       }


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-837157923


   ASCII85InputStream.read(byte[] data, int offset, int len). Row 182. Maybe logical OR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846418217


   Maybe it is impossible but... CosStream.createView: if filterList.size() will be zero then the input will not be closed. And you able to write this loop without if statement. Just move try-finally to up, make memory allocation only in  the loop.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-830846915


   There is strange behavior. The method PdCalGray.toBGB returns clon in first case and returns not clon in second case.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846984084


   FileSystemFontProvider.addType1Font. Row 760. If type1.getName() will return null (check row 754) it can produce a NPE. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-830580796


   It looks like the SortMask.SoftPaintContext.dispose method should call context.dispose (release the resources of the field) or it needs a comment inside the dispose method.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-813262045


   > > PDDeviceCMYK.INSTANCE is not final
   > 
   > That is for code like this, which uses the official Adobe CMYK icc profile (with incompatible license)
   > 
   > ```
   >         // https://www.adobe.com/support/downloads/iccprofiles/iccprofiles_win.html
   >         PDDeviceCMYK.INSTANCE = new PDDeviceCMYK()
   >         {
   >             @Override
   >             protected ICC_Profile getICCProfile() throws IOException
   >             {
   >                 return ICC_Profile.getInstance("USWebCoatedSWOP.icc");
   >             }
   >         };
   > ```
   
   ok. Thanks for explanation.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-811246650


   1 CFFExpertCharset has static instance (singletone) but uses static field (CFF_EXPERT_CHARSET_TABLE). Make field non static?
   2 CFFExpertSubsetCharset has static instance (singletone) but uses static field (CFF_EXPERT_SUBSET_CHARSET_TABLE). Make field non static?
   Maybe, will be better if you will use local variable in static initializer instead of field.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-964417948


   > 
   > 
   > Hi @THausherr! Please, check the COSDictionary.getDictionaryString method. It looks like there are error on line 1372 ("if (base instanceof COSStream)" in the wrong place).
   
   A COSStream is a COSDictionary that has a stream (assuming that you think it's suspicious that the check is inside a block that is for COSDictionaries)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-964823754


   Hi, @THausherrI found one strange method. COSInteger.equals. Why it has no isValid method checks?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-969903630


   > Yeah maybe it should be done. But isValid is about big values that are outside of allowed range, so maybe they are equal to another value value in a different type, so maybe that is the reason it isn't checked.
   
   Yes may be but shoud
   
   > > Hi, @THausherrI found one strange method. COSInteger.equals. Why it has no isValid method checks?
   > 
   > Yeah maybe it should be done. But isValid is about big values that are outside of allowed range, so maybe they are equal to another value value in a different type, so maybe that is the reason it isn't checked.
   
   I dont know the range of these values. Can be COSInteger be valid with max (or min) long value? If yes, IMHO, then we need isvalid check in the equals method.
   You are using the boolean field and long field. If max long and min long are valid you able to check to max and min values of the COSInteger instances (static fields). You will delete boolean field this way. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-798947542


   There should be only one tag before BMC. If there are more, then it's probably trash and yes, it makes sense to keep the last one.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-803539343


   > PDFTextStripper.processPage
   
   Yes it is suspicious. Sadly I don't know what this code is doing so I prefer not to touch it for now. It's probably related to "beads", an obscure PDF feature that was [broken for years in PDFBox without anybody noticing](https://issues.apache.org/jira/browse/PDFBOX-3110). That code segment is not hit by any tests.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-867467534


   COSOutputStream.close. Row 131. The buffer field will not be closed because a RandomAccessInputStream not overrides close method. Or don't you need try-with resources?
   COSOutputStream.close. Row 146. The writer field will not be closed because a RandomAccessOutputStream not overrides close method.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-867467534


   COSOutputStream.close. Row 131. The buffer field will not be closed because a RandomAccessInputStream not overrides close method. Or don't you need try-with resources?
   COSOutputStream.close. Row 146. The buffer field will not be closed because a RandomAccessOutputStream not overrides close method.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-790869979


   May I ask you why the CFFTable class has a tag with a space?
   Code:
   
   `public static final string TAG = "CFF ";`


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-850418503


   > 1. maybe. I'll do something at a later time
   > 2. no, bytes being not null means that this has worked in the past.
   > 3. yes, but this would mean a broken project.
   
   okay


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-855241295


   PDAcroForm.flatten(List<PDField>, boolean). The contentStream will not be closed if an exception will be thrown.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-913181285


   > This one I won't do (except the comment, at a later time). The performance win is minimal, maybe none, but the code gets much longer. This method is called once on a PDF. There will usually be at least 16 bytes unless the PDF is broken.
   
   I changed this code not for speed but better memory usage.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835820459


   > Oops, you're of course right. I reviewed the rest to find the creation of these arrays and these are new Point2D values each time, but who knows what a future developer will do, so I'll clone the elements.
   
   i would do the same for for CubicBesierCurve, CoonsPatch, PDMechBasedShadingType. Maybe you will find more.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-831778824


   ![image](https://user-images.githubusercontent.com/67366451/116979570-d6fb5200-accd-11eb-8f6f-1471007a9657.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-792553474


   For a FileSystemFontProvider.readTrueTypeFont. If will be exception from ttc.getFontByName then ttc will not be closed. Same for getOTFFont.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-792022230


   @THausherr: "The one in TrueTypeCollection.java I don't agree. It looks kind of weird that two parser objects are generated but only one is used."
   Me: Actually, it depends of algorithm. If we need the same parser every time, then we can add one parameter instead of two to the getFontAtIndex method. I don't know this algorithm well, so if you think we only need one parser for each method (getFontByName and processAllFonts), that's ok.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846986510


   FileSystemFontProvider.addTrueTypeFontImpl. Same problem for NPE.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-850548729


   > Re ASCII85Filter / DCT filter - the problem is that I would have to close the top level object and I no longer have it. No harm is done currently, it's just "not a good style".
   
   You haven't need to close the top level object. It is problem of end user (programmer with uses this library). You able to write documentation, code comments, code examples. The right or responsibility to learn Java and pdfbox lies with the end user of the library. But, actually, I thought, we closed this question because you decided not change anything.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-834681664


   > CosInputStream.flush. Why not if buffer is null then super.flush()?
   
   Please clarify


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-855397628


   PDPageLabels.LabelGenerator.next. Row "while (label.lastIndexOf(0) != -1)". This loop removes all symbols in a string before the null bytes including them. If we need to do it why not use label.indexOf without loop? And do we need to remove null bytes from the end of the string or all null bytes in the string (check comment to the code)?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-905787547


   > 
   > 
   > StandardSecurityHandler.prepareForDecryption. Line 153: encryption.get__String__FilterName()?
   
   good observation! fixed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-906643827


   > I don't see the problem - it retrieves the last element, which is a name. That is checked earlier.
   
   Yes. Now I see


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-904772908


   > 
   > 
   > FDFJavaScript.getDoc. This method returns null or empty map but not map with keys and values.
   
   Thanks, will be fixed in https://issues.apache.org/jira/browse/PDFBOX-5264 .


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-904772530


   > 
   > 
   > Hi, @THausherr! MapBackedScriptFeature.equals. Line 105. This is right fields comparing? The maps compares keys in the equals method and you compares keys with type List by reference but not value.
   
   Maybe. I can't really decide, not knowing the intent. This has to be decided by whoever wants to finish that part. (it is about complex scripts, e.g. in India, Thailand, etc)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-791997929


   The one in TrueTypeCollection.java I don't agree. It looks kind of weird that two parser objects are generated but only one is used.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-904546337


   FDFJavaScript.getDoc. This method returns null or empty map but not map with keys and values.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-833230736


   > This is an error case. ("Step and YStep may be either positive or negative but shall not be zero.")
   
   Sorry. I not understood. Did make the bug or not?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-834255227


   JPEGFactory.retrieveDimensions(ByteArrayInputStream). Same as Filter.findImageReader


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-813329836


   > Yes it is a tricky code segment. I prefer to wait until somebody brings up such a file. IIRC one of the problems is that we can't throw an exception. Another problem is that we can't perform worse than Adobe Reader.
   
   understood


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-788232197


   I understood. Best regards!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-855397628


   PDPageLabels.LabelGenerator.next. Row "while (label.lastIndexOf(0) != -1)". This loop removes all symbols in a string before the null bytes including them. If we need to do it why not use label.indexOf without loop?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-868308933


   > Isn't `buffer` closed in the `finally` block? Same for `filteredBuffer`. Or did I misunderstand this?
   
   Yes, buffer will be closed in inner finally block (because of swapping). As I understand, only last filteredBuffer will be closed, because outer finally block located outside the loop. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-868308933


   > Isn't `buffer` closed in the `finally` block? Same for `filteredBuffer`. Or did I misunderstand this?
   
   Yes, buffer will be closed in inner finally block (because of swapping). As I understand, only last filtered buffer will be closed, bacause outer finally block located outside the loop. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-813298597


   > > Do you want to say there is difference in memory usage for a field and local variable? But it is only one additional parameter.
   > 
   > Yes I assume that. It's pushed on the stack with each call. It's twice as much parameters than before, and this in a code segment that is highly recursive. (page tree)
   
   Okay. Let's use the old version, but what about a potential NPE in PageIterator.next (sanitizeType(next) - next variable can be null)? Looks like it is this issue: https://issues.apache.org/jira/projects/PDFBOX/issues/PDFBOX-4034?filter=allopenissues


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-796018402


   You are not forgotten, but I've been busy. Might take a few days.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-856119473


   > Your code removes all after 0 and including this 0, but that is different to what the current code does. We don't need this at this time.
   
   Your code does the same:
       public static void main(String[]args) {
           String s=cutNullBytes(new byte[]{ 1,2,3,0,0,0,0,0,5,4,5,4,6,0,0,0},false);
           String s1=cutNullBytes(new byte[]{ 1,2,3,0,0,0,0,0,5,4,5,4,6,0,0,0},true);
           String s2=cutNullBytes(new byte[]{ 1,2,3,5,4,5,4,6,0,0,0,0},false);
           String s3=cutNullBytes(new byte[]{ 1,2,3,5,4,5,4,6,0,0,0,0},false);
           System.out.println(s.equals(s1));
           System.out.println(s2.equals(s3));
       }
   
       private static String cutNullBytes(byte[]bytes, boolean useLastIndexOf) {
           String label = new String(bytes);
           System.out.println("label length: " + label.length() + " label value: " + label);
   
           if (useLastIndexOf) {
               while (label.lastIndexOf(0) != -1)
               {
                   label = label.substring(0, label.length() - 1);
               }
           }
           else {
               int index = label.indexOf(0);
               if (index > -1)
                   label = label.substring(0, index);
           }
   
           System.out.println("label length: " + label.length() + " label value: " + label);
   
           return label;
       }


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846971349


   Why COSParser.bruteForceSearchTriggered is not private?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-834680450


   > JPEGFactory.retrieveDimensions(ByteArrayInputStream). Same as Filter.findImageReader
   
   thanks, fixed, but in 3.0 only.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846166741


   IdentityFilter and CCITTFaxFilter extends Filter
   IdentityFilter.encode(InputStream, OutputStream, COSDictionary). The InputStream will not be closed.
   CCITTFaxFilter.encode(InputStream, OutputStream, COSDictionary). The InputStream will be closed.
   The input parameter will not be closed if an exception is thrown.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-904885292


   > > WinAnsiEncoding
   > 
   > > WinAnsiEncoding constructor. Check comment (276) and creation of index (277).
   > 
   > So what's the problem? I don't see a contradiction.
   No problem. I thought means decimal systemin in comment
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-890298627


   Hi, @THausherr!
   1 Maybe this is not a problem and it will not be. I found that COSArray.setString can set null as a value. Some methods, such as COSArray.indexOf, use an objects collection with no null checks.
   2 PDAnnotation.getBorder. If border.size() >= 3 don't need to "create a copy to avoid changing the PDF"?
   3 static final fields of PDRectangle are modifiable (LETTER, LEGAL etc.)
   4 TrailerValidationProcess.checkTrailersForLinearizedPDF14. Row 96. A last variable can be null.
   5 PreflightConfiguration.SHADING_PATTERN_PROCESS. Typo in value. Double 'd'.
   6 RubberStampAnnotationValidator. Maybe wrong comment: BudderStampAnnotation or RubberStampAnnotation
   7 class Decrypt. Wrong command name (row 41: "decrpyt")
   8 DomXmpParser.manageDefinedType (382, 394, 419). A parseLiDescription method able to return null. Can ast variable be null here.
   9 TiffSchema.IMAGE_LENGHT instead of IMAGE_LENGTH
   
   I remember about preflight but these notes were written before information about preflight removing.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-831765070


   ![image](https://user-images.githubusercontent.com/67366451/116977018-92ba8280-acca-11eb-8248-4d52e4403e16.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846615543


   DecodeResult.DEFAULT uses COSDictionary field. Why DEFAULT field uses modifiable COSDictionary and colorSpace? The COSDictionary can be changed via getParameters method. The colorSpace can be changed via setColorSpace method.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-847183618


   I committed two changes re: FileSystemFontProvider; one of those you mentioned I didn't because it's already been taken care of, see at the bottom of addTrueTypeFontImpl.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-831358700


   Thanks, fixed
   https://issues.apache.org/jira/browse/PDFBOX-5183
   and also inspired me for
   https://issues.apache.org/jira/browse/PDFBOX-5184


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835121361


   > TilingPaint.getAnchorRect. Can the pattern.getBBox method return null in this case (using in the method getAnchorRect)?
   
   Yeah it could be. I've fixed this and this brought back the optimization that you suggested earlier.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-868950765


   There is simplified version of COSOutputStream.close:
      @Override
       public void close() throws IOException
       {
           try
           {
               if (buffer != null)
               {
                   try
                   {
                       // apply filters in reverse order
                       for (int i = filters.size() - 1; i >= 0; i--)
                       {
                           try (InputStream unfilteredIn = new RandomAccessInputStream(buffer))
                           {
                               if (i == 0)
                               {
                                   /*
                                    * The last filter to run can encode directly to the enclosed output
                                    * stream.
                                    */
                                   filters.get(i).encode(unfilteredIn, out, parameters, i);
                               }
                               else
                               {
                                   RandomAccess filteredBuffer = scratchFile.createBuffer();
                                   try (OutputStream filteredOut = new RandomAccessOutputStream(filteredBuffer))
                                   {
                                        filters.get(i).encode(unfilteredIn, filteredOut, parameters, i);
                                   }
                                   finally
                                   {
                                       buffer.close();
                                       buffer = filteredBuffer;
                                   }
                               }
                           }
                       }
                   }
                   finally
                   {
                       buffer.close();
                       buffer = null;
                   }
               }
           }
           finally
           {
               super.close();
           }
       }


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835458649


   > > Maybe it is not bug but a little strange. You changed the PDCIEDictionaryBasedColorSpace.setWhitePoint and setBlackPoint. Do getters of these whitepoint and blackpoint good? You set PDTristimulus but you get cosarray.
   > 
   > Not sure what you mean (both setter and getter have PDTristimulus as parameter and return value), but setItem() can be called with a COSObjectable and will then "do the right thing" i.e. get the COSObject and work with that one.
   
   Thanks. I got.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-833525117


   PDLab.toRGBImage. You don't have to do the subtraction every time. Speed is more important here.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835075692


   COSOutputStream.flush. @THausherr, you need use a buffer if super is null. To revert null check.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-968134524


   Hm. You changed all 'this' to 'the' for PDType1Font and added a throw statement with same message with 'this'.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1077945750


   > Hi, @THausherr! I found a comment of SequenceRandomAccessRead.createView method was removed.
   
   Which one / when? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-818918758


   Re PDViewerPreferences: The same problem also applies to the document information. And probably to other parts of the document, e.g. fonts or images. Yes all of this carries risks, mostly closing the documents before saving. I think that this is more a documentation problem, i.e. save first, then close, and don't reuse.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-868308933


   > Isn't `buffer` closed in the `finally` block? Same for `filteredBuffer`. Or did I misunderstand this?
   
   Yes, buffer will be closed in inner finally block.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-847041909


   ASCII85Filter.decode. Should the InputStream encoded be closed inside? The same for DCTFilter, ASCII85Filter.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-806748128


   PDFStreamParser.parseNextToken. Default statement. You check if an trimmed operator length bigger than zero. If condition is true you call "Operator.getOperator(operator)" but use non trimmed value. May be this version will be better:
               default:
                   // we must be an operator
                   String operator = readOperator().trim();
                   if (operator.length() > 0)
                   {
                       return Operator.getOperator(operator);
                   }


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-788132306


   tnx for fast respond. I'll try to fix it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-997278739


   Good find. I think this can be fixed by changing the code to be like this:
   ```
               else if (counter > 0)
               {
                   counter = 0;
                   source.seek(position + 1);
                   position = -1L;
                   //continue;
               }
   ```
   The code is used only when we do a brute force search on broken PDFs. If a PDF would contain "%%%EOF" then "%%EOF" wouldn't be found with the existing code. The perfect solution would be 
   https://en.wikipedia.org/wiki/Knuth%E2%80%93Morris%E2%80%93Pratt_algorithm
   but that would need more time.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-964417948


   > 
   > 
   > Hi @THausherr! Please, check the COSDictionary.getDictionaryString method. It looks like there are error on line 1372 ("if (base instanceof COSStream)" in the wrong place).
   
   A COSStream is a COSDictionary which has a stream.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-796882339


   Thanks. I've been busy too


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1077959801


   > > Hi, @THausherr! I found a comment of SequenceRandomAccessRead.createView method was removed.
   > 
   > Which one / when?
   
   Heh! It was mine.
   
   ![image](https://user-images.githubusercontent.com/67366451/159992284-3aae29ff-5f55-431d-a75a-a37cd1ee1cd0.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1069430606


   Hi, @THausherr! PDFCloneUtility.checkForRecursion. No need to convert a parent parameter to COSBase. It can be object for faster checking.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835468485


   A ShadesTriangle constructor. The Point2d is reference type and it is mutable.
    So, it is not deep clonning or need comment. Potential problem: Patch.getShadedTriangles.Mayme this array of Point2d uses only for reading. But why not some immutable point?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-788221570


   Thanks, I committed everything except the memory leak. The problem is that I don't know for sure if the object is still used. The tests pass, but we don't really have much tests in FDF / XFDF. And the parser was modified to be "on demand" in the trunk, which means that input may still be accessed after parsing (however I don't know if this applies to FDF / XFDF, or only for PDF).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-806093557


   > labels.length is always > 0 because there's always at least 1 page in a document. If not, there's something rotten anyway.
   
   I got a little carried away by the debugger. I used a check for an empty document, because the library allows you to create blank documents


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835482082


   Pdfbox, javadoc. Maybe a dot was lost after 'www': http://wwwimages.adobe.com/content/dam/Adobe/en/devnet/pdf/pdfs/PDF32000_2008.pdf


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-836413815


   FDFAnnotationCaret.setSymbol. Maybe it is not a bug but looks weird. A code says: you able to put any symbol or null but we add 'P' or 'None'. If that's correct why not use boolean or enumeration as parameter?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-834294357


   If ShadingContext is public why dispose is package private?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-867467534


   COSOutputStream.close. Row 131. A COSOutputStream.buffer field will not be closed because a RandomAccessInputStream not overrides close method. Or don't you need try-with-resources?
   COSOutputStream.close. Row 146. A filteredBuffer variable will not be closed because a RandomAccessOutputStream not overrides close method.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-791999125


   The one about the double-checked-locking I'll do after the next release. This is a very sensitive topic.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835768285


   > COSDictionary.getObjectFromPath method. If the first object found is sufficient, you need to add two break operators to the loop.
   
   No, this is a bit like XPath. The real problem here is that the method has no test and no error handling 😡


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-882028500


   > This change makes code less readable and uses a boolean parameter which breaks the "do one thing" rule. (Yes we probably break this rule a lot, but there's no need to add more). It does save 3 or 4 bytes, but I doubt that this makes a difference.
   > https://twitter.com/unclebobmartin/status/1114137614377005057
   
   Hi, @THausherr! I tried to allocate less memory. If you will purpose another way it will be good. I mean, if you will use public constructor (instead of private constructor and static method it will be simplier). As you see I got one second faster test results.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-868305576


   Isn't `buffer` closed in the `finally` segment? Same for `filteredBuffer`. Or did I misunderstand this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-849890816


   > * AcroFormOrphanWidgetsProcessor.resolveNonRootField. Row 192. Does variable 'field' can be null here?
   > * FF3ByteSource.getBytes. Row 485. Does getFontDescriptor().getFontFile3() can return null here?
   > * Version.getVersion(). The Version.class.getResourceAsStream(PDFBOX_VERSION_PROPERTIES) statement can return null.
   
   Hi, @THausherr! What do you think about these things? If it doesn't matter, that's okay.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-805885019


   when I uncommented COSArrayListTest.addToList and launched it, it was green


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov closed pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov closed pull request #107:
URL: https://github.com/apache/pdfbox/pull/107


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-803531725


   ok


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-841778448


   COSArrayList. When you add the first element, you refresh the parentDict field (set to "array"). When you clear the COSArrayList instance, you do the same (set to null).
   I can't understand why you don't do the same for removeAll, remove, retainAll for the empty collection case (need to check "array.isEmpty ()")? Maybe it cannot be empty. I mean, without refreshing the parentDict field for these cases. What if only first item will be removed (need to refresh a value from parentDict)?
   Second question is why should COSArray.removeAll method not work like COSArrayList.removeAll? I mean the first part of the COSArrayList.removeAll method where the items are removed from the COSArray. You are converting COSArray elements using the getObject method, why don't you use this code in COSArray.removeAll?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-792026731


   @THausherr : The one about the double-checked-locking I'll do after the next release. This is a very sensitive topic.
   Me: this is a known pattern to reduce the use of synchronized context (since synchronization is slow). This is useful for methods that can be called many times. And all changed methods are public, so the programmer can call them multiple times.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835042564


   Thanks, fixed in https://issues.apache.org/jira/browse/PDFBOX-5188


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-863476067


   > Please don't submit code style changes. The style is mostly relevant for new code.
   
   I changed only because licenses had too many stars in the comments. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-854903623


   > Why make it protected? They're both private and very different. To make this less confusing, I'm renaming them to something very different.
   
   it is a way


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846415688


   > Re filter: My understanding is that the caller should close streams unless the javadoc tell something else. So I did search a bit, and found places in COSStream and PDStream where this isn't done. I'll create an issue.
   > 
   > Re COSArrayList: that one I prefer not to touch myself, but I think there's an issue about it in JIRA, if not, please create one.
   
   Filter: I believe if a resource  (stream) has been created outside it should be closed outside. Ok, you fixed it.
   CosArrayList: you wrote me about problems with this class and sent a link: https://issues.apache.org/jira/browse/PDFBOX-4669. I found these weird rows of code in this class. Ok. I'll sent these results to that issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-868305576


   Isn't `buffer` closed in the `finally` block? Same for `filteredBuffer`. Or did I misunderstand this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-850548729


   > Re ASCII85Filter / DCT filter - the problem is that I would have to close the top level object and I no longer have it. No harm is done currently, it's just "not a good style".
   
   You haven't need to close the top level object. It is problem of end user (programmer with uses this library). You able to write documentation, code comments, code examples. the right or responsibility to learn Java and pdfbox lies with the end user of the library. But, actually, I thought, we closed this question because you decided not change anything.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-798947639


   COSArrayList.equals strange implementation. There may be a problem if the user (programmer) sends the COSArray instance as equals parameter.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-803540879


   > > PDFTextStripper.processPage
   > 
   > Yes it is suspicious. Sadly I don't know what this code is doing so I prefer not to touch it for now. It's probably related to "beads", an obscure PDF feature that was [broken for years in PDFBox without anybody noticing](https://issues.apache.org/jira/browse/PDFBOX-3110). That code segment is not hit by any tests.
   
   Maybe you able to add this note to Issue Tracker to investigate in future.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-890381198


   > PDAnnotation.getBorder. If border.size() >= 3 don't need to "create a copy to avoid changing the PDF"?
   
   No. The "create copy" is a workaround to be able to work with a broken PDF without modifying it. It's not perfect, but we try not to change PDFs when retrieving stuff. I have improved the javadoc.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-890394495


   re 1, 5, 6, 7, 8, 9: fixed, thanks
   re 3: 🙈 for now 😂
   re 4: will wait until it happens. Not sure if this can ever happen.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-808439680


   I won't do the "if (resourceAsStream == null)" removal, we just had a user who set up a part of the project (without the resources) and got weird error messages and claimed that our examples weren't working at all. I'll also revert the similar commit already done.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846984084


   FileSystemFontProvider.addType1Font. Row 760. If type1.getName() will return null (check row 754) it can produce NPE. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-837086601


   @THausherr , will you fix this cosname-test? 'PDFBOX-5190: revert changes as those were too strict', but not test.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-861149415


   Thanks, fixed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-850088834


   1) maybe. I'll do something at a later time
   2) no, bytes being not null means that this has worked in the past.
   3) yes, but this would mean a broken project.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-834681438


   > Re Patch: thanks, fixed. Re "ShadingContext is public why dispose package private"? Don't know. It's possible that somebody wanted to reference the object without needing to dispose. Maybe I should remove "public" in the upcoming 3.0 version and wait for complaints.
   
   Note: Children of ShadingContext uses an interface PaintContext with has a dispose method too. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835124674


   > Maybe it is not bug but a little strange. You changed the PDCIEDictionaryBasedColorSpace.setWhitePoint and setBlackPoint. Do getters of these whitepoint and blackpoint good? You set PDTristimulus but you get cosarray.
   
   Not sure what you mean (both setter and getter have PDTristimulus as parameter and return value), but setItem() can be called with a COSObjectable and will then "do the right thing" i.e. get the COSObject and work with that one.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-803535762


   > COSArrayList.equals strange implementation. There may be a problem if the user (programmer) sends the COSArrayList instance as equals parameter.
   
   Comment probably belongs to https://issues.apache.org/jira/browse/PDFBOX-4669


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-806051549


   Thanks, I've forwarded the test thing and changed the JIRA issue.
   
   Re PDFDebugger, I'll do just a few of the changes. (I'm hopelessly behind) I'll do the "static" thing assuming this is a hint in some IDEs (not mine). Performance improvements for PDFDebugger aren't really important (unless they're noticeable by humans). This is a GUI tool that is used mostly by PDFBox developers who don't have access to Adobe tools. Optimizations are more relevant for text extraction and rendering. There I'd say every millisecond counts, because these add up.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-803531725


   ok. Tank you for explanation. I was not fooled if it looks like this:
   `                actuals.add(COSDictionaryMap
                           .convertBasicTypesToMap(
                                         (COSDictionary)array.getObject(i)));`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-847010729


   SequenceRandomAccessRead.createView. This is an UnsupportedOperationException.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846596307


   ![hashmap_cannot_contain_double_keys](https://user-images.githubusercontent.com/67366451/119270185-2addc480-bc04-11eb-9507-b28d4665636c.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846614634


   > private constructor of PDStream. If input will be null then NPE will be thrown inside a row "IOUtils.copy(input, output);" (137). And redurant closing of stream. IMHO, the input should be closed outside.
   
   Thanks, but I've removed the null check instead. The javadoc mentions that the methods do close the input stream.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-833227055


   > "cannot use shading if shading function is null" - most triangle based shadings don't have a function. The better solution might be to pass the function.
   
   As I understood, additionally you are making map.put(...). If you found better solution it is good. I tought about to devide this code to two functions: with shading and without.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-813083249


   PDDeviceCMYK.INSTANCE is not final


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-808439680


   I won't do the "if (resourceAsStream == null)" removal, we just had a user who set up a part of the project (without the resources) and got weird error messages and claimed that our examples weren't working at all. I'll also revert the similar commit already done.
   
   I see the exception was done at the request of another user:
   https://issues.apache.org/jira/browse/PDFBOX-4990


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-827584201


   Hi, @THausherr! I researched the PDFCloneUtility code and found one detail. This class has two fields as collections (cloneVersion and clonedValues). I understood these collections never clears. Is all their content really needed for clonning or you can clear some items for specific level of stack or before some method, or after getting an item from collection?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835468485


   A ShadesTriangle constructor. The Point2d is reference type and it is mutable.
    So, it is not deep clonning or need comment. Potential problem: Patch.getShadedTriangles.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-819692863


   No, although there is a cloning class, but I prefer not to use it because it could create an incredible bloat. Not so much because these two dictionaries, but because of the rest.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-792830324


   I understood. I thought about triple allocation of memory for the File object. You can allocate memory before the if statement.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-791567468


   Thanks. Googbye


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835820459


   > Oops, you're of course right. I reviewed the rest to find the creation of these arrays and these are new Point2D values each time, but who knows what a future developer will do, so I'll clone the elements.
   
   i would do the same for for CubicBesierCurve, CoonsPatch, PDMechBasedShadingType. Maybe you will find more. And, please, think about unmodifiable point. In this case you will not clone anything.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-836390762


   > The one in TrueTypeCollection.java I don't agree. It looks kind of weird that two parser objects are generated but only one is used.
   
   You able to make the parsers as fields but allocate memory before the loops if need (if they are null).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-808661852


   > I won't do the "if (resourceAsStream == null)" removal, we just had a user who set up a part of the project (without the resources) and got weird error messages and claimed that our examples weren't working at all. I'll also revert the similar commit already done.
   > 
   > I see the exception was done at the request of another user:
   > https://issues.apache.org/jira/browse/PDFBOX-4990
   
   ok. But perhaps you will write a more clear version. You are creating a buffered stream even resource stream is null. You close resource stream and then close buffered stream, but the second stream is wrapper for the first. This way, there is no need to close the resource stream because it will be closed automatically. I know your version of code is shorter and the sonarcloud will be quiet but we write code for programmers not for sonarcloud. Maybe the sonarcloud will find not closed resources (it will be wrong clause) but I'm suggesting this template for all plases (version for Standard14Fonts.loadMetrics):
   
       private static void loadMetrics(String fontName) throws IOException
       {
           String resourceName = "/org/apache/pdfbox/resources/afm/" + fontName + ".afm";
           InputStream resourceAsStream = PDType1Font.class.getResourceAsStream(resourceName);
   
           if (resourceAsStream == null)
           {
               throw new IOException("resource '" + resourceName + "' not found");
           }
   
           try(InputStream afmStream = new BufferedInputStream(resourceAsStream))
           {
               AFMParser parser = new AFMParser(afmStream);
               FontMetrics metric = parser.parse(true);
               FONTS.put(fontName, metric);
           }
       }


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-847215141


   COSParser.parseTrailer. If a nextLine variable will start with a space character then a nextLine.startsWith("trailer") return false. Сan the nextLine start with a space? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-847010729


   SequenceRandomAccessRead.createView. It is UnsupportedOperationException.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr removed a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr removed a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-964417948


   > 
   > 
   > Hi @THausherr! Please, check the COSDictionary.getDictionaryString method. It looks like there are error on line 1372 ("if (base instanceof COSStream)" in the wrong place).
   
   A COSStream is a COSDictionary that has a stream (assuming that you think it's suspicious that the check is inside a block that is for COSDictionaries)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov removed a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov removed a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-964415738


   Hi @THausherr! Please, check the COSDictionary.getDictionaryString method. It looks like there are error on line 1372 ("if (base instanceof COSStream)" in the wrong place).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-804360640


   Should the reader read the raster or not?
   ![image](https://user-images.githubusercontent.com/67366451/112052073-55a39200-8b5b-11eb-9f2b-2647933972c2.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-907833215


   Hi, @THausherr!
   My two observations.
   1 Is it not too math to enlarge ScratchFileBuffer.pageIndexes array length twice (ScratchFileBuffer.addPage)?
   2 PDVisibleSigBuilder.createTemplate. If this method will be called twice then previous template (PDDocument) not be closed. You able to store link to that template in a PDVisibleSigBuilder. If this link will be same as template field then it will be closed.
   3 PDPageFitRectangleDestination. SetLeft method calls growToSize 3 to set index 2. This logic broken for methods setBottom and setRight. It looks like size should be different. 
   PDPageXYZDestination.PDPageXYZDestination. The same problem.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835776179


   > I fixed the checkbox thing, thanks. I don't understand what you mean with the Point2d thing. What I can tell:
   > 
   > * it works fine (there are almost no public tests due to file copyright and slight differences with different jdks, but there are many rendering tests that I do locally)
   > * that package is extremely performance sensitive
   > * the Point2D class is partly used to put coordinates in a set
   
   ShadedTriangle.constructor. You are clonning an array but NOT the array values (Point2d). So will be several arrays with same instances of points. If programmer will decide change some point in an array (for example to change x-coord) then the point will be changed for all arrays. I not found using of setters of Point2d but it is potential problem.
   Point2d[]ar1 = ar2.clone();
   system.out.print(ar1==ar2); //expecting false
   system.out.print(ar1[0]==ar2[0]);//expecting FALSE


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-833227055


   > "cannot use shading if shading function is null" - most triangle based shadings don't have a function. The better solution might be to pass the function.
   
   As I understood, additionally you are making map.put(...). If you found better solution it is good. I tought about to devide this code to two functions: with shading and without (dont remember details)... or this null check was redurant.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-830848548


   > There is strange behavior. The method PdCalGray.toBGB returns clon in first case and returns not clon in second case.
   
   It's always a new object. In the first case a clone is done. In the second case it's the result of CIEXYZ.toRGB, in the third case it's a new object.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-797325489


   A bit strange implementation of COSArray.getInt (...) and others. If the index is larger than the size of the array, then defaultValue will be returned instead of throwing an IndexOutOfRangeException. Maybe defaultValue should be returned if obj is not COSNumber?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov closed pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov closed pull request #107:
URL: https://github.com/apache/pdfbox/pull/107


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-796882339


   Thanks. I've been busy for too


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-992764988


   > The first is a slight memory optimization but makes the code less readable. The second wouldn't work because the PDRectangle constructor takes width/height and not coordinates as parameters.
   
   Hi, @THausherr! Thank you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-997237347


   My guess is that the comment is misleading. Should probably rather be a text like "should not happen, likely broken PDF". The code is related to
   https://issues.apache.org/jira/browse/PDFBOX-3891
   which has bad PDFs.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-997377146


   > I agree that the method is also flawed because the offset isn't set to the starting point - 1 after a fail, but I doubt troubles would happen as long as we only search for %%EOF and startxref.
   > 
   > ```
   >         RandomAccessRead source = new RandomAccessReadBuffer(new byte[0]);
   >         PDFParser parser = new PDFParser(source, null, null, null, null);
   >         byte [] buf = { '%', '%', '%', '%', 'E', 'O', 'F', 'O', 'F' };
   >         int result = parser.lastIndexOf(EOF_MARKER, buf, buf.length);
   >         System.out.println("result: " + result);
   > ```
   > 
   > Can you come up with a failing test?
   
   I didn't find any matching string for lastIndexOf


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1077993440


   > If PDDocument.saveIncrementalForExternalSigning will be called several times then signingSupport instance will be createad again but released only once (PDDocument.close())
   
   There's no reason to call it several times IIRC.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1045995533


   > Hi, @THausherr! Method PDTerminalField.importFDF (103). Can the getWidgets() method return an emply list for this case (probably an IndexOutOfBoundsException)?
   
   Thanks, fixed as part of https://issues.apache.org/jira/browse/PDFBOX-5379


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835468485


   A ShadesTriangle constructor. The Point2d is reference type
    So, it is not deep clonning or need comment. Example: Point2Patch.getShadedTriangles.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-848083747


   Please discuss this in those issues. They are both self-assigned to Andreas so maybe he has already done something.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-813274532


   > Do you want to say there is difference in memory usage for a field and local variable? But it is only one additional parameter.
   
   Yes I assume that. It's pushed on the stack with each call. It's twice as much parameters than before, and this in a code segment that is highly recursive. (page tree)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-882731846


   Rather not, this would mean even more work, for tiny advantages.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-836760988


   BlendComposite.BlendCompositeContext. A field hints is redurant.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-882731846


   Rather not, this would mean even more work, for tiny advantages.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-836378435


   > I realize that we don't have to clone the points. These aren't changed in PDFBox, and maybe people outside shouldn't change them. I'm too influenceable in my effort to produce the best code. I'll sleep over this and will then possibly revert the previous change and just add some comment about not modifying the points.
   
   I believe, thr best code it is code without comments and a big documentation. Programmers hate to read comments and docs. We as developers of library or contributors able to forget some details of an implementation. I understand a good code without comments it is something wery seldom. Ok. It is your choise.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-834253976


   CosInputStream.flush. Why not if buffer is null then super.flush()?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-789061160


   @lehmi  Thank you.
   
   @valerybokov I committed the FDF part, but used the interface type on the left side of the assignment, see [here](https://stackoverflow.com/questions/19611908/) why (although not really important here)
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-855626092


   Re PDPageLabels: my understanding of the code is that it removes the 0 bytes at the end, and only these, see https://issues.apache.org/jira/browse/PDFBOX-1047 . The problem string there is <33000000> so there are three 0 bytes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-830849462


   ![concurrent_map_in_the_syncronyzed_method](https://user-images.githubusercontent.com/67366451/116823136-782dbf80-ab8b-11eb-964f-6fcc73966664.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-837160661


   > PDResources.getExtGState. row 249. The indirect variable cannot be null here or it is acceptable value? The same for getShading, getPattern, getProperties, getXObject. Actually null is available value for HashMap but it means you refresh a cache.
   
   You're right, this makes no sense. I'll fix this tomorrow or later. (busy week)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr removed a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr removed a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-847489084


   > BaseParser.readStringNumber. A buffer.length() cannot be bigger than Integer.MAX_VALUE (max length of any java array).
   
   The name of that constant is a bit confusing, MAX_LENGTH_LONG is 19.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846608397


   PDFPrintable.print (row 194). Suggestion: create a field instead of calling the method  for document.getNumberOfPages(). Reason is TODO in PDDocument: REPLACE most calls to this method with BELOW method


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-836393107


   > > Note: Children of ShadingContext uses an interface PaintContext with has a dispose method too.
   > 
   > Yes it is kindof weird that ShadingContext isn't an implementation of PaintContext. I remember that I did some huge refactoring of the excellent work of my student at the end of GSoC 2014 and that there were difficulties. Maybe I should try it (ShadingContext as an implementation of PaintContext) and see what happens.
   
   Few years ago I was teacher too.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-890298627


   Hi, @THausherr!
   1 Maybe this is not a problem and it will not be. I found that COSArray.setString can set null as a value. Some methods, such as COSArray.indexOf, use an objects collection with no null checks.
   2 PDAnnotation.getBorder. If border.size() >= 3 don't need to "create a copy to avoid changing the PDF"?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-811246650


   1 CFFExpertCharset has static instance (singletone) but uses static field (CFF_EXPERT_CHARSET_TABLE). Make field non static?
   2 CFFExpertSubsetCharset has static instance (singletone) but uses static field (CFF_EXPERT_SUBSET_CHARSET_TABLE). Make field non static?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-819336208


   > I don't see how this would make it faster, a type is probably also some sort of number in a table. And the way you're setting currentReturnType is a side effect. The fonts aren't really that problematic re speed (they are parsed only once), and these are type 1 fonts, these appear mostly in old PDFs.
   
   thanks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-792830324


   I understood. I thought about triple allocation of memory for the File object. You can allocate memory before the if statement and when need to change file.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-798947639


   COSArray.equals strange implementation. There may be a problem if the user (programmer) sends the COSArray instance as equals parameter.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-790790933


   My bad. I will revoke it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-790536900


   Hi! I've made a few changes to fontbox. I hope they will be useful to you.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-869173102


   filter() only returns a new image if dest is null ("dest color converted from src or a new, converted BufferedImage if dest is null") so it doesn't matter.
   
   Btw please don't bother with preflight. This subproject will be removed in 4.0. The EU funded VeraPDF is also open source and has more features.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-848075698


   Hi, @THausherr ! What if I'll devide cosparser from the bruteforce parsing code and xref parsing code  (https://issues.apache.org/jira/projects/PDFBOX/issues/PDFBOX-5032, https://issues.apache.org/jira/projects/PDFBOX/issues/PDFBOX-5031). I know, it is for next version but I can do it. I able to create 3 or 4 classes insted of 1.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-882731846


   Rather not, this would mean even more work, for tiny advantages.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-905238386


   StandardSecurityHandler.prepareForDecryption. Line 153: encryption.get__String__FilterName()?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835111981


   Actually, I dont know why John Hewson overrided flush to empty. But a flush.method calls inside a super.close (and doing nothing with this overriding). But that is not fear implementation. So, I believe, we need flush a super stream if we used it before.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-997327723


   I agree that the method is also flawed because the offset isn't set to the starting point - 1 after a fail, but I doubt troubles would happen as long as we only search for %%EOF and startxref.
   
   ```
           RandomAccessRead source = new RandomAccessReadBuffer(new byte[0]);
           PDFParser parser = new PDFParser(source, null, null, null, null);
           byte [] buf = { '%', '%', '%', '%', 'E', 'O', 'F', 'O', 'F' };
           int result = parser.lastIndexOf(EOF_MARKER, buf, buf.length);
           System.out.println("result: " + result);
   ```
   Can you come up with a failing test?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1046265181


   I have one little proposition. The method PDPage.getAnnotations(AnnotationFilter) returns instance of COSArrayList. The PageDrawer uses it inside drawPage method (reading). We able to create additional method to reduce memory usage for this case. Method:
   
   //The idea: don't use COSArrayList because it is expensive. Use ArrayList.
   
       /**
        * This will return a list of the annotations for this page.
        *
        * @param annotationFilter the annotation filter provided allowing to filter out specific annotations
        * @return List of the PDAnnotation objects, never null.
        *
        * @throws IOException If there is an error while creating the annotation list.
        */
       public List<PDAnnotation> getAnnotationsList(AnnotationFilter annotationFilter) throws IOException
       {
           COSArray annots = page.getCOSArray(COSName.ANNOTS);
           if (annots == null)
           {
               return Collections.EMPTY_LIST;
           }
           List<PDAnnotation> actuals = new ArrayList<>();
           for (int i = 0; i < annots.size(); i++)
           {
               COSBase item = annots.getObject(i);
               if (item != null)
               {
                   PDAnnotation createdAnnotation = PDAnnotation.createAnnotation(item);
                   if (annotationFilter.accept(createdAnnotation))
                   {
                       actuals.add(createdAnnotation);
                   }
               }
           }
           return actuals;
       }
   
   Maybe, this additional method will be better if you use it instead of PDPage.getAnnotations() for some cases. The code:
       public List<PDAnnotation> getAnnotationsList() throws IOException
       {
   	return getAnnotationsList(annotation -> true);
       }


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-947908126


   Thanks, fixed in PDFBOX-5298. It's the first time I hear about this class.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1073034788


   Hi, @THausherr! There is changed COSParser.findString method with implemented KMP algorithm. Please, check it and commit
   [COSParser.txt](https://github.com/apache/pdfbox/files/8309587/COSParser.txt)
   .


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-964420992


   > A COSStream is a COSDictionary that has a stream (assuming that you think it's suspicious that the check is inside a block that is for COSDictionaries)
   
   Thank, you. Now I see.  COSStream is a COSDictionary.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-964823754


   Hi, @THausherr! I found one strange method. COSInteger.equals. Why it has no isValid method checks?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-855397628


   PDPageLabels.LabelGenerator.next. Row "while (label.lastIndexOf(0) != -1)". This loop removes all symbols in a string before the null bytes including them. If we need to do it why not use label.indexOf without loop? And do we need to remove null bytes from the end of the string, or all null bytes in the string?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846415688


   > Re filter: My understanding is that the caller should close streams unless the javadoc tell something else. So I did search a bit, and found places in COSStream and PDStream where this isn't done. I'll create an issue.
   > 
   > Re COSArrayList: that one I prefer not to touch myself, but I think there's an issue about it in JIRA, if not, please create one.
   
   Filter: I believe if a resource  (stream) has been created outside it shoud be closed outside. Ok, you fixed it.
   CosArrayList: you wrote me about problems wirh this class and sent a link: https://issues.apache.org/jira/browse/PDFBOX-4669. I found these weird rows of code in this class. Ok. I'll sent these results to that issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1064909170


   Hi, @THausherr ! CosWriter.prepareIncrement. If cosObjectKey variable can be null getObjectFromPool method will throw NPE. A COSDocument.getObjectsByType method has no this check too. There is NPE or redurant null check?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-813324291


   Yes it is a tricky code segment. I prefer to wait until somebody brings up such a file. IIRC one of the problems is that we can't throw an exception. Another problem is that we can't perform worse than Adobe Reader.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-792813489


   COSDictionary.getDictionaryString (row 1526). It looks like the toList method is unnecessary.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-792830324


   I understood. I thought about double allocation of memory for the File object. You can allocate memory before the if statement.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-834686269


   > > > CosInputStream.flush. Why not if buffer is null then super.flush()?
   > > 
   > > 
   > > Please clarify
   > 
   > class CosInputStream{
   > //,,,
   > public void flush(){
   > if (buffer==null) super.flush();}
   > //,,,
   > }
   
   Sorry. CosOutputStream


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-841778448


   COSArrayList. When you add the first element, you refresh the parentDict field (set to "array"). When you clear the COSArrayList instance, you do the same (set to null).
   I can't understand why you don't do the same for removeAll, remove, retainAll for the empty collection case (need to check "array.isEmpty ()")? Maybe it cannot be empty. I mean, without refreshing the parentDict field for these cases. What if only first item will be removed (need to refresh a value from parentDict)?
   Second question is why should COSArray.removeAll method not work like COSArrayList.removeAll? I mean the first part of the COSArrayList.removeAll method where the items are removed from the COSArray. You are converting COSArray elements using the getObject method, why don't you use this code in COSArray.removeAll?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-867467534


   COSOutputStream.close. Row 131. The input field will not be closed because a RandomAccessInputStream not overrides close method. Or don't you need try-with resources?
   COSOutputStream.close. Row 146. The writer field will not be closed because a RandomAccessOutputStream not overrides close method.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-817756433


   I created a PDF document with 2 pages. Additionally, I added an instance of PDViewerPreferences. After splitting the document, I got their PDViewerPreferences and compared their COSObjects. The links are the same. So, if I want to change the PDViewerPreferences for one of the created documents, I need to clone the PDViewerPreferences and set them to the document. No cloning method found from COSDictionary.
   
   The Method PDDocumentCatalog.getViewerPreferences sends a link to PDViewerPreferences constructor instead of clonned object link. I should say, sometimes link is better than clone. For example, if you save document to a drive (PDFSplit.call).
   [pdviewerprefs.txt](https://github.com/apache/pdfbox/files/6296536/pdviewerprefs.txt)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-803537124


   About the PDFDebugger.DocumentOpener it is ok. It is just a bigger memory usage.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-791557441


   I'm done for now. Thanks again. More later or tomorrow.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-868950765


   There is simplified version of COSOutputStream.close:
   `   @Override
       public void close() throws IOException
       {
           try
           {
               if (buffer != null)
               {
                   try
                   {
                       // apply filters in reverse order
                       for (int i = filters.size() - 1; i >= 0; i--)
                       {
                           try (InputStream unfilteredIn = new RandomAccessInputStream(buffer))
                           {
                               if (i == 0)
                               {
                                   /*
                                    * The last filter to run can encode directly to the enclosed output
                                    * stream.
                                    */
                                   filters.get(i).encode(unfilteredIn, out, parameters, i);
                               }
                               else
                               {
                                   RandomAccess filteredBuffer = scratchFile.createBuffer();
                                   try (OutputStream filteredOut = new RandomAccessOutputStream(filteredBuffer))
                                   {
                                        filters.get(i).encode(unfilteredIn, filteredOut, parameters, i);
                                   }
                                   finally
                                   {
                                       buffer.close();
                                       buffer = filteredBuffer;
                                   }
                               }
                           }
                       }
                   }
                   finally
                   {
                       buffer.close();
                       buffer = null;
                   }
               }
           }
           finally
           {
               super.close();
           }
       }`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846418217


   Maybe it is impossible but... CosStream.createView: if filterList.size() will be zero then the input will not be closed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-792022230


   @THausherr: "The one in TrueTypeCollection.java I don't agree. It looks kind of weird that two parser objects are generated but only one is used."
   Me: Actually, it depends of algorithm. If we need the same parser every time, then we can add one parameter instead of two to the getFontAtIndex method. I don't know this algorithm well, so if you think we only need one parser for each method (getFontByName and processAllFonts), that's ok. The goal is to reduce the number of memory allocations because it is slow. Therefore it looks strange.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-798947443


   Looks like CertInformationCollector.CertSignatureInformation and PDFDebugger.DocumentOpener should be static.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846987521


   FileSystemFontProvider.addTrueTypeFont. Row 621. I think, toLowerCase() needed because extension can be in upper case.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-899034394


   > COSParser.parseTrailer. If a nextLine variable will start with a space character then a nextLine.startsWith("trailer") return false. Сan the nextLine start with a space?
   
   No, because there's a loop that ends when hitting a "t".


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-904865473


   > WinAnsiEncoding
   
   
   
   > 
   > 
   > WinAnsiEncoding constructor. Check comment (276) and creation of index (277).
   
   So what's the problem? I don't see a contradiction.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-890298627


   Hi, @THausherr!
   1 Maybe this is not a problem and it will not be. I found that COSArray.setString can set null as a value. Some methods, such as COSArray.indexOf, use an objects collection with no null checks.
   2 PDAnnotation.getBorder. If border.size() >= 3 don't need to "create a copy to avoid changing the PDF"?
   3 static final fields of PDRectangle are modifiable (LETTER, LEGAL etc.)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-847489084


   > BaseParser.readStringNumber. A buffer.length() cannot be bigger than Integer.MAX_VALUE (max length of any java array).
   
   The name of that constant is a bit confusing, MAX_LENGTH_LONG is 19.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846615543


   DecodeResult.DEFAULT uses COSDictionary field. Why DEFAULT field uses modifiable COSDictionary and colorSpace? The COSDictionary can be changed via getParameters method. The colorSpace can be changed via setColorSpace method.I think the DEFAULT value should always be the same.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-806055173


   labels.length is always > 0 because there's always at least 1 page in a document. If not, there's something rotten anyway.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-854450805


   Type1CharString contains private method named closepath. Type2CharString contains private method named closePath. Maybe, able we to rename Type1CharString.closepath to Type1CharString.closePath and make it protected?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-855397628


   PDPageLabels.LabelGenerator.next. Row "while (label.lastIndexOf(0) != -1)". This loop removes all symbols in a string before the null bytes including them. If we need to do it why not use label.indexOf without loop? And do we need to remove null bytes from the end of the string, or all null bytes in the string (check comment to the code)?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-833227055


   > "cannot use shading if shading function is null" - most triangle based shadings don't have a function. The better solution might be to pass the function.
   
   As I understood, additionally you are making map.put(...). If found better solution it is good. I tought about to devide this code to two functions: with shading and without.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-811246650


   1 CFFExpertCharset has static instance (singletone) but uses non static field (CFF_EXPERT_CHARSET_TABLE). Make field non static?
   2 CFFExpertSubsetCharset has static instance (singletone) but uses non static field (CFF_EXPERT_SUBSET_CHARSET_TABLE). Make field non static?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-833536337


     PDCIEDictionaryBasedColorSpace.setBlackPoint. The comment says "As this is a required field
     this null should not be passed into this function.". The code says "null will be processed without problems".


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-791567468


   Thanks. Goodbye


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-847184327


   > I committed two changes re: FileSystemFontProvider; one of those you mentioned I didn't because it's already been taken care of, see at the bottom of addTrueTypeFontImpl.
   
   yes. I discovered it too.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846608397


   PDFPrintable.print (row 194). Suggestion: create a field instead of calling the method  for document.getNumberOfPages(). Reason is TODO in PDDocument: REPLACE most calls to this method with BELOW method. Additionally check row 202. Maybe field PDPageTree will be better than current version.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-789169947


   Thanks, I think we're done here. If yes, then please close the PR.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-819388535


   > Re PDViewerPreferences: The same problem also applies to the document information. And probably to other parts of the document, e.g. fonts or images. Yes all of this carries risks, mostly closing the documents before saving. I think that this is more a documentation problem, i.e. save first, then close, and don't reuse.
   
   So, do you planning to add cloning?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-868306265


   > RandomAccessInputStream and RandomAccessOutputStream are simple wrapper classes and close isn't needed at all. Maybe we should just add an empty implementation of close to document that. Right now try-with-resources avoids sonar warnings
   
   Yes, I've seen several cases where streams could not be closed. But why are you using the trick for COSOutputStream.close? You use fake try-with-resources statement to avoid sonar warnings, then you use swapping of streams to close right stream. Why not use try-with-resources with the correct stream and without RandomAccessOutputStream? I understand, maybe this code matches the documentation, but it is too confusing, IMO.
   ![2021-06-25 104909](https://user-images.githubusercontent.com/67366451/123392310-95887400-d5a5-11eb-8c6c-324ad0d77286.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-834665252


   Re Patch: thanks, fixed. Re "ShadingContext is public why dispose package private"? Don't know. It's possible that somebody wanted to reference the object without needing to dispose. Maybe I should remove "public" in the upcoming 3.0 version and wait for complaints.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846406659


   Re filter: My understanding is that the caller should close streams unless the javadoc tell something else. So I did search a bit, and found places in COSInputStream, COSStream and PDStream where this isn't done. I'll create an issue.
   
   Re COSArrayList: that one I prefer not to touch myself, but I think there's an issue about it in JIRA, if not, please create one.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-846406659


   Re filter: My understanding is that the caller should close streams unless the javadoc tell something else. So I did search a bit, and found places in COSStream and PDStream where this isn't done. I'll create an issue.
   
   Re COSArrayList: that one I prefer not to touch myself, but I think there's an issue about it in JIRA, if not, please create one.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-803531725


   ok. Tank you for explanation. I was not fooled if it looks like this:
   `                actuals.add(COSDictionaryMap<br>
                           .convertBasicTypesToMap(
                                         (COSDictionary)array.getObject(i)));`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-868306265


   > RandomAccessInputStream and RandomAccessOutputStream are simple wrapper classes and close isn't needed at all. Maybe we should just add an empty implementation of close to document that. Right now try-with-resources avoids sonar warnings
   
   Yes, I've seen several cases where streams could not be closed. But why are you using the trick for COSOutputStream.close? You use fake try-with-resources statement to avoid sonar warnings, then you use swapping of streams to close right stream. Why not use try-with-resources with the correct stream and without RandomAccessOutputStream? I understand, maybe this code matches the documentation, but it is too confusing, IMO.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] lehmi commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
lehmi commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-868238678


   RandomAccessInputStream and RandomAccessOutputStream are simple wrapper classes and close isn't needed at all. Maybe we should just add an empty implementation of close to document that. Right now try-with-resources avoids sonar warnings


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov removed a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov removed a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835482082


   Pdfbox, javadoc. Maybe a dot was lost after 'www': http://wwwimages.adobe.com/content/dam/Adobe/en/devnet/pdf/pdfs/PDF32000_2008.pdf


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-789169947


   Thanks, I think we done here. If yes, then please close the PR.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-867467534


   COSOutputStream.close. Row 131. A COSOutputStream.buffer field will not be closed because a RandomAccessInputStream not overrides close method. Or don't you need try-with resources?
   COSOutputStream.close. Row 146. A filteredBuffer variable will not be closed because a RandomAccessOutputStream not overrides close method.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-906594590


   PDColor.PDColor(COSArray, PDColorSpace). Line 58. Index is zero based (was -1 lost?).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-847230592


   > ASCII85Filter.decode. Should the InputStream encoded be closed inside? The same for DCTFilter, ASCII85Filter.
   
   No, this is done by the caller, see earlier text here


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-969903630


   > > Hi, @THausherrI found one strange method. COSInteger.equals. Why it has no isValid method checks?
   > 
   > Yeah maybe it should be done. But isValid is about big values that are outside of allowed range, so maybe they are equal to another value value in a different type, so maybe that is the reason it isn't checked.
   
   I dont know the range of these values. Can COSInteger be valid with max (or min) long value? If yes, IMHO, then we need isvalid check in the equals method.
   You are using the boolean field and long field. If max long and min long are valid you able to check to max and min values of the COSInteger instances (static fields). You will delete boolean field this way. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-959920979


   You have changed the PDColorSpace class. It looks like the ColorConvertOp field might be static? Why do you need many instances of ColorConvertOp for each PDColorSpace instance? (PDFBOX-5310: create ColorConvertOp only once)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-959920979


   You have changed the PDColorSpace class. It looks like the ColorConvertOp field might be static? Why do you need many instances of ColorConvertOp for each PDColorSpace instance?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-959920979






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-968209106


   Thanks, fixed. I missed that one but did it correctly in the 2.0 branch.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1077954790


   If PDDocument.saveIncrementalForExternalSigning will be called several times then signingSupport instance will be createad again but released only once (PDDocument.close())


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1077915935


   Hi, @THausherr! I found a comment of SequenceRandomAccessRead.createView method was removed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1077959801


   > > Hi, @THausherr! I found a comment of SequenceRandomAccessRead.createView method was removed.
   > 
   > Which one / when?
   
   Oops! It was mine.
   
   ![image](https://user-images.githubusercontent.com/67366451/159992284-3aae29ff-5f55-431d-a75a-a37cd1ee1cd0.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-997232062


   PDAcroFormFromAnnotsTest.testFromAnnots3891ValidateFont (261)
   ![image](https://user-images.githubusercontent.com/67366451/146649656-ea4f9a2d-1ff9-41ea-aaee-43b561676446.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-803536866


   public class CertInformationCollector
   {
   	//.,.
   	public static class CertSignatureInformation
       {
   		//...
   	}
   	//...
   }


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-837090191


   PDResources.getExtGState. row 249. The indirect variable cannot be null here or it is acceptable value? The same for getShading, getPattern, getProperties, getXObject. Actually null is available value for HashMap but it means you refresh a cache.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835123235


   > COSOutputStream.flush. @THausherr, I think, you need use a super if buffer is null. To revert null check.
   
   Thanks. The risk of early morning work.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-827769059


   I don't know and I prefer not to test this, because in the worst case we would either not clone enough, or have doubles. This is a very sensitive part of the code, I had some hard time around xmas 2018 fixing bugs. The structure of PDF isn't a pure tree, there's a lot of references going around, e.g. between the structure tree and the pages.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-947862659


   Hi, @THausherr ! I found one strange detail in a PageExtractor class. An interesting process is going on inside the extraction method. In the loop we calling the importPage method. Inside this method we changing the cropbox, mediabox and rotation for an imported page. We doeing the same in the PageExtractor.extract method. Maybe you will remove unneeded initialisations of the cropbox etc. Also, the importPage method notifies us about resources but we doeing right operation with resources in the PageExtractor.extract method (inherited resources of source document are not imported to...). 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-850540365


   Re ASCII85Filter / DCT filter - the problem is that I would have to close the top level object and I no longer have it. No harm is done currently, it's just "not a good style".


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-835075692


   COSOutputStream.flush. @THausherr, you need use a super if buffer is null. To revert null check.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-960491720


   > > Hi, @THausherr! You have changed the PDColorSpace class. It looks like the ColorConvertOp field might be static? Why do you need many instances of ColorConvertOp for each PDColorSpace instance? (PDFBOX-5310: create ColorConvertOp only once)
   > 
   > Yes this was changed for speed. The effects are amazing for files with many tiny images. There seems to be some caching going on, from looking at the source code. I assumed that this caching would be per entity, so each entity should keep its own colorspace. A common ColorConvertOp for all would mean synchronization and possibly loss of caching, even if it doesn't happen, then maybe in older versions.
   
   Understood. Thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-959920979






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-959920979


   Hi, @THausherr! You have changed the PDColorSpace class. It looks like the ColorConvertOp field might be static? Why do you need many instances of ColorConvertOp for each PDColorSpace instance? (PDFBOX-5310: create ColorConvertOp only once)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-969853797


   > 
   > 
   > Hi, @THausherr ! The classes JBIG2Filter (decode method) and COSWriter (getDataToSig method) uses a SequenceInputStream class with constructor with Vector creation. The Vector is syncronized collection and, you know, it is slow. Is it necessary to use this constructor but not another?
   
   This is a one time thing for saving or for decoding an unusual image type so the effect will be negligible. Even oracle hasn't changed it. Using the other constructor would make it less readable.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] THausherr edited a comment on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
THausherr edited a comment on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-804595994


   Ouch. Thanks for finding this. Proposed improved code:
   ```
   
       protected static ImageReader findImageReader(String formatName, String errorCause) throws MissingImageReaderException
       {
           Iterator<ImageReader> readers = ImageIO.getImageReadersByFormatName(formatName);
           ImageReader reader;
           while (readers.hasNext())
           {
               reader = readers.next();
               if (reader != null && reader.canReadRaster())
               {
                   return reader;
               }
           }
           throw new MissingImageReaderException("Cannot read " + formatName + " image: " + errorCause);
       }
   
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements

Posted by GitBox <gi...@apache.org>.
valerybokov commented on pull request #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-830848979


   > > There is strange behavior. The method PdCalGray.toBGB returns clon in first case and returns not clon in second case.
   > 
   > It's always a new object. In the first case a clone is done. In the second case it's the result of CIEXYZ.toRGB, in the third case it's a new object.
   
   okay


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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