[jira] [Updated] (LOG4J2-154) ThreadContext performance improvement: shallow copies for reads, deep copies for writes

["Ralph Goers (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/LOG4J2-154?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ralph Goers updated LOG4J2-154:
-------------------------------

    Attachment: LOG4J2-154.patch

Based on what I saw from your code I have created my own version of a patch for this issue. Normally I follow the Commit-Then-Review model and have others review my commits after they are done.  But in this case I would really like some feedback before I apply the patch to see if anyone can spot problems with it.
                
> ThreadContext performance improvement: shallow copies for reads, deep copies for writes
> ---------------------------------------------------------------------------------------
>
>                 Key: LOG4J2-154
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-154
>             Project: Log4j 2
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 2.0-beta3
>            Reporter: Remko Popma
>         Attachments: LOG4J2-154.patch, LOG4J2-154-patch-DefaultThreadContextMap.txt, LOG4J2-154-patch-ThreadContextMap.txt, LOG4J2-154-patch-ThreadContext.txt
>
>
> Currently, every time a Log4jLogEvent object is created, a deep copy is made of both the context map and the context stack. However, expected usage is that only a few objects are pushed onto the stack or put in the context map, while the number of LogEvents is in the thousands or millions.
> Essentially, there are far more reads than writes, so using a copy-on-write mechanism should give us better performance.
> Example context map put: deep copy (expensive but rare)
>     public static void put(String key, String value) {
>         if (!useMap) {
>             return;
>         }
>         Map<String, String> map = localMap.get();
>         Map<String, String> copy = null;
>         if (map == null) {
>             copy = new HashMap<String, String>();
>         } else {
>             copy = new HashMap<String, String>(map);
>         }
>         copy.put(key, value);
>         localMap.set(copy);
>     }
> Example context stack push: deep copy (expensive but rare)
>     public static void push(String message) {
>         if (!useStack) {
>             return;
>         }
>         ContextStack stack = localStack.get();
>         ContextStack copy = null;
>         if (stack == null) {
>             copy = new ThreadContextStack();
>         } else {
>             copy = stack.copy();
>         }
>         copy.push(message);
>         localStack.set(copy);
>     }
> Now, when the Log4jLogEvents are created, they just call ThreadContext.getImmutableContext and getImmutableStack. These methods return an unmodifiable wrapper around the most recent copy.
> Example for context map:
>     public static Map<String, String> getImmutableContext() {
>         Map<String, String> map = localMap.get();
>         return map == null ? EMPTY_MAP : Collections.unmodifiableMap(map);
>     }
> Example for context stack:
>     public static ContextStack getImmutableStack() {
>         ContextStack stack = localStack.get();
>         return stack == null ? EMPTY_STACK : new ImmutableStack(stack.asList());
>     }
> Note that ThreadContext.ThreadContextStack needs to be changed to contain an ArrayList rather than extend it, to facilitate making both deep mutable copies and shallow immutable copies.
>     private static class ThreadContextStack implements ContextStack {
>         private static final long serialVersionUID = 5050501L;
>         private List<String> list;
>         public ThreadContextStack() {
>             list = new ArrayList<String>();
>         }
>         /**
>          * This constructor uses the specified list as its internal data
>          * structure unchanged. It does not make a defensive copy.
>          */
>         public ThreadContextStack(List<String> aList) {
>             list = aList; // don't copy!
>         }
>         /**
>          * This constructor copies the elements of the specified collection into
>          * a new list. Changes to the specified collection will not affect this
>          * {@code ThreadContextStack}.
>          */
>         public ThreadContextStack(Collection<String> collection) {
>             list = new ArrayList<String>(collection);
>         }
>         public void clear() {
>             list.clear();
>         }
>         public String pop() {
>             int index = list.size() - 1;
>             if (index >= 0) {
>                 String result = list.get(index);
>                 list.remove(index);
>                 return result;
>             }
>             throw new NoSuchElementException("The ThreadContext stack is empty");
>         }
>         public String peek() {
>             int index = list.size() - 1;
>             if (index >= 0) {
>                 return list.get(index);
>             }
>             return null;
>         }
>         public void push(String message) {
>             list.add(message);
>         }
>         public int getDepth() {
>             return list.size();
>         }
>         public List<String> asList() {
>             return list;
>         }
>         public void trim(int depth) {
>             if (depth < 0) {
>                 throw new IllegalArgumentException(
>                         "Maximum stack depth cannot be negative");
>             }
>             while (list.size() > depth) {
>                 list.remove(list.size() - 1);
>             }
>         }
>         /**
>          * Returns a deep copy of this stack.
>          */
>         public ContextStack copy() {
>             return new ThreadContextStack(new ArrayList<>(list));
>         }
>     }

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

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