[
https://issues.apache.org/jira/browse/LOG4J2-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13655663#comment-13655663
]
Ralph Goers commented on LOG4J2-154:
------------------------------------
Normally CTR is OK. I didn't do that for this because the changes could have a
wide impact and I wanted other eyes to review it. I'm pretty comfortable with
the changes to the ContextMap but the ContextStack concerned me. Personally, I
have no idea why anyone would want to use the ContextStack but since it was
part of Log4j 1 I didn't feel I could drop it.
Feel free to run with this.
> 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-unsubscr...@logging.apache.org
For additional commands, e-mail: log4j-dev-h...@logging.apache.org
