You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "pjfanning (via GitHub)" <gi...@apache.org> on 2024/01/20 11:03:25 UTC

[I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

pjfanning opened a new issue, #994:
URL: https://github.com/apache/incubator-pekko/issues/994

   We use commons-io as a test dependency but not as a dependency of our core libs.
   
   We've been adding some small perf enhancements but this is going to be more contentious because it involves adding a dependency.
   
   If we did add this dependency, we could use UnsynchronizedByteArrayInputStream/UnsynchronizedByteArrayOutputStream instead of java.io ByteArrayInputStream/ByteArrayOutputStream which has synchronized blocks. 
   
   UnsynchronizedByteArrayOutputStream also has an efficient toInputStream which is better than getting a byte[] from ByteArrayOutputStream and creating a new ByteArrayInputStream. https://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/output/UnsynchronizedByteArrayOutputStream.html#toInputStream()
   
   Another alternative is to copy the code we want into pekko-actor instead of having a jar dependency.
   
   Wdyt @mdedetrich @kerr @samueleresca @nvollmar 
   


-- 
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: notifications-unsubscribe@pekko.apache.org.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on issue #994:
URL: https://github.com/apache/incubator-pekko/issues/994#issuecomment-1902077889

   And easy to port to scala js and scala native


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on issue #994:
URL: https://github.com/apache/incubator-pekko/issues/994#issuecomment-1902072097

   > > And furture more , we can just translate that code to Scala with the license header annotated.
   > 
   > If we do copy the code that we want into pekko-actor then we should leave it as Java. Our build supports building Java code and it would be easier to get fixes and improvements from commons-io if we do so. The code will probably be more efficient in Java anyway.
   
   There is an advantage to using Scala, it will her picked up by the Scala 2 inliner (the Scala 2 inliner doesn't work with Java sources)


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning closed issue #994: add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents)
URL: https://github.com/apache/incubator-pekko/issues/994


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on issue #994:
URL: https://github.com/apache/incubator-pekko/issues/994#issuecomment-1902239903

   We can't copy the JDK code (incompatible license) but the Spring class looks like a good one - simpler implementation but has the features we need. https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/util/FastByteArrayOutputStream.html Writing an InputStream that works with that should be straightforward. 


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on issue #994:
URL: https://github.com/apache/incubator-pekko/issues/994#issuecomment-1902069366

   > And furture more , we can just translate that code to Scala with the license header annotated.
   
   If we do copy the code that we want into pekko-actor then we should leave it as Java. Our build supports building Java code and it would be easier to get fixes and improvements from commons-io if we do so. The code will probably be more efficient in Java 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.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "samueleresca (via GitHub)" <gi...@apache.org>.
samueleresca commented on issue #994:
URL: https://github.com/apache/incubator-pekko/issues/994#issuecomment-1902131486

   Some considerations:
   
   - `commons.io` looks great from a vulnerabilities perspective. It seems that the past ~10 releases don't have any vulnerability discovered.
   -  At the same time,we would like to add the package for taking advantage of a single feature for the sake of some potential perfromance gains. 
   - There have been git activity on both  `UnsynchronizedByteArrayInputStream`, `UnsynchronizedByteArrayOutputStream`  classes but I don't spot any massive rework / controversial change that might cause headache in future (I haven't check any deps of `UnsynchronizedByteArray*Stream` classes)
   
   My opinion is that we can at least try to internalize the `UnsynchronizedByteArrayInputStream` and `UnsynchronizedByteArrayOutputStream` code. If it is too much effort (either internalization or further maintenance it) then we can take a step back and add the dependency. 
   
   On the the Java vs Scala rewrite point: how many classes / components that are dependency of `UnsynchronizedByteArray*Stream` do we need need to internalize? I'm tempted to understand first if internalization is a route, and postpone the decision of Java vs Scala rewrite
   


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on issue #994:
URL: https://github.com/apache/incubator-pekko/issues/994#issuecomment-1932772615

   my testing indicates that we probably don't need 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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on issue #994:
URL: https://github.com/apache/incubator-pekko/issues/994#issuecomment-1902259779

   I have an experimental project where I have the basic classes that we need. https://github.com/pjfanning/io-experiment
   
   Still in Java but can theoretically be ported to Scala.


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on issue #994:
URL: https://github.com/apache/incubator-pekko/issues/994#issuecomment-1902223169

   I have to sat, I was want to port Betty's buffer to pure scala too, so does the jctools.


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on issue #994:
URL: https://github.com/apache/incubator-pekko/issues/994#issuecomment-1902068690

   I think we should not add another dependnecy to actor-core.
   ![image](https://github.com/apache/incubator-pekko/assets/501740/643fafa3-2fc1-4327-8022-5af135b91b84)
   
   It currently only has two dependencies, and where do you plan to use it? I was want to have jctools in as mailbox for akka-typed/akka-stream actor, but I saw cats-effect just maintain it in their code.
   
   So If we just use a little poison of the lib, we can just source in some code, especially if user want to use pekko on android, they would want to reduce the dependencies.


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on issue #994:
URL: https://github.com/apache/incubator-pekko/issues/994#issuecomment-1902068948

   And furture more , we can just translate that code to Scala with the license header annotated.


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [I] add commons-io dependency or cherry pick some code (has more performant code in some cases than built-in java equivalents) [incubator-pekko]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on issue #994:
URL: https://github.com/apache/incubator-pekko/issues/994#issuecomment-1902208410

   I'm not against copying the code we need into Pekko. The latest commons-io code has major tentacles. The new builder brings in all sorts of other classes in commons-io. We might be able to write a simple one of our own.


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org