nfsantos commented on code in PR #2146:
URL: https://github.com/apache/jackrabbit-oak/pull/2146#discussion_r1984585531


##########
oak-store-spi/src/main/java/org/apache/jackrabbit/oak/plugins/memory/PropertyStates.java:
##########
@@ -89,7 +90,7 @@ public static PropertyState createProperty(
             String name, Iterable<Value> values)
             throws RepositoryException {
         int type = PropertyType.STRING;
-        Value first = Iterables.getFirst(values, null);
+        Value first = StreamUtils.toStream(values).findFirst().orElse(null);

Review Comment:
   This might be introducing a performance regression. `Iterables.getFirst` 
does not allocate any object, while `StreamUtils.toStream()` creates an object 
internally. And the code path of `Iterables.getFirst` is much shorter, so 
potentially faster than the Guava equivalent. This method, `createProperty()`, 
is likely performance critical, so I'm worried this could significantly degrade 
performance of some parts of Oak. 
   In general, I'm concerned about all these patches removing Guava. We have 
tests for correctness (and even with those tests, these patches have broken the 
build several times). But we do not have any performance regression tests, so 
these patches may be degrading the performance without us realising.



-- 
This is an automated message from the 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: oak-dev-unsubscr...@jackrabbit.apache.org

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

Reply via email to