[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #532: Replace loop with bulk method.

[GitBox <gi...@apache.org>]

carterkozak commented on a change in pull request #532:
URL: https://github.com/apache/logging-log4j2/pull/532#discussion_r658181865



##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/util/TypeUtil.java
##########
@@ -53,9 +54,7 @@ private TypeUtil() {
     public static List<Field> getAllDeclaredFields(Class<?> cls) {
         final List<Field> fields = new ArrayList<>();
         while (cls != null) {
-            for (final Field field : cls.getDeclaredFields()) {
-                fields.add(field);
-            }
+            fields.addAll(Arrays.asList(cls.getDeclaredFields()));

Review comment:
       I don't think it's entirely clear which one will perform better, but I agree with @vy that the original is likely to outperform addAll.
   
   The benefit of addAll is that the receiving collection doesn't need to resize the underlying array multiple times, however ArrayList.addAll(Collection) calls Collection.toArray(), and Arrays.asList(arr) uses an implementation which produces a copy of the underlying array on each call. That's expensive, but we're deep enough into hot methods that C2 may be able to help us out by avoiding the duplicate array allocation (potentially including the getDeclaredFields() array).




-- 
This is an automated message from the 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