CODEWITHSUNDEEP
javamultithreadingoptimizationconcurrencyjvm
damat-perdigannat
damat-perdigannat

Reputation: 5960

Thread safety of the plus operator for Strings, optimizations included

This post says that a += b is the equivalent of

a = new StringBuilder()
    .append(a)
    .append(b)
    .toString();

Let's say I have this code:

public class MultiThreadingClass extends SomeThirdPartyClassThatExtendsObject{
    public void beginmt(String st) throws IOException {
        //st is a thread number
        st = new File("c:\\somepath").getCanonicalPath()+"\\"+st;
        System.out.println(st);
    }
}

Assume that beginmt runs multiple times simultaneously (with thread numbers 1 to 15500) on a single instance of MultiThreading class. Could there be instances such that it could print the following i.e. some thread numbers are lost and some numbers are doubled?

c:\somepath\2
c:\somepath\1
c:\somepath\1
c:\somepath\4
c:\somepath\5
c:\somepath\6
c:\somepath\7
c:\somepath\8
c:\somepath\8
c:\somepath\10
...

Edit:

Will it be safe to say that the + operator won't get into some unsafe publication issue? I'm thinking the StringBuilder could be optimized into something that resembles an instance variable in which case it could be unsafely published.

Edit 2:

As far as the JLS, the abovementioned post, and a similar class file for the above code are checked, the StringBuilders to be used seem to have to get contained within different stackframes. However, I'd still like to check whether some form of aggressive optimization could cause the StringBuilders to be replaced by a centralized StringBuilder in some way. This sounds possible as it sounds logical for optimizers to optimize when it predicts that an object is just implemented in a non-constant way when in fact such object could be constant.

Found stringopts.cpp but haven't found the time yet to fully check it. I'm hopefully looking for answers involving details of this source file.

Edit 3:

I'm still looking for answers that include code on aggressive inlining for mutable objects.

Upvotes: 5

Views: 1659

Answers (6)

Paul Ianas
Paul Ianas

Reputation: 501

I have to partially disagree. This sentence is incomplete / missleading:

"If instead st was a member variable of that class, instead of being passed as an argument, and was incremented - that's a different story."

What matters in the original example is that the expression on the right is a rvalue. If it were not, the outcome would have been different. I will explain a bit.

So yes, Strings are immutable, and beginmt() receives a final reference to a String and this means a final reference to an immutable heap memory area. JVM will make a copy of this final reference and whatever you do inside the beginmt(), it is done on this copy, and this copy, immediately after the string is modified (st = ...), will point to another memory area. Now the point is that this final heap memory area has no pointer to it, because it is created inside the method and it seems that nothing points to it. Well, almost! The JVM may intern the string and if another thread points to the same value as the interned value it could be possible that they would actually share the same heap address. Now having a race condition exactly here is very hard to detect, so I will make a synthetic example to illustrate what could happen in case the expression on the right is a lvalue (induced by JVM's String intern-al):

public class AnExample { private static final int N = 20000;

private static class Foo {
    static HashMap<String, Foo> foos = new 
            HashMap<String, Foo>(N);

    static synchronized Foo createInstance(String i) {
        if (foos.containsKey(i))
            return foos.get(i);
        foos.put(i, new Foo(i));
        return foos.get(i);
    }

    String i;

    private Foo(String i) {
        this.i = i;
    }

    Foo inc() {
        synchronized(Foo.class){ 
            i += "1";
            return createInstance(i);
        }
    }

    @Override public String toString() {
        return i;
    }
}

private static class Bar {
    public void bar(Foo st) {
        st = st.inc();
        System.out.println(st);
    }
}

public static void main(String... args) {
    final Bar cucu = new Bar();
    for (int i = 0; i < N; i++) {
        final Foo st = Foo.createInstance(i + "");
        new Thread(new Runnable(){
            @Override public void run() {
                cucu.bar(st);
            }
        }).start();
    }

    try {
        Thread.sleep(10000);
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
}

}

This will produce about 40% duplicates (I have less than 10100 unique values). Note that in the bar() method st = some_expression(st) In my example I intentionally produce a lvalue just to show what might happen in case the JVM will intern the expression and it happens to have a reference to that (which is sent back in another thread to the same method).

The conclusion is that your code is not correct because "st is not a member variable of that class" and "st becomes a local, copied reference", etc - but because the expression on the right is a rvalue.

Upvotes: 1

bvdb
bvdb

Reputation: 24760

Each thread will always have individual StringBuilder instances. Thread-safety is no issue when threads don't share instances.

So, the following simple method ...

public class MyThreadSafeClass
{
  public String myMethod(String field1, String field2, String field3)
  {
     return field1 + field2 + field3;
  }
}

... will be compiled to use a local StringBuilder.

public class MyThreadSafeClass
{
  public String myMethod(String field1, String field2, String field3)
  {
     return new StringBuilder(field1).append(field2).append(field3).toString();
  }
}

Each time the method is entered, a new StringBuilder instance is created. This instance is only used withing the scope of this thread.

You are correct however that StringBuilders are not always thread-safe. (see below) If multiple threads start calling the saveEvent method, they may be using the builder simultaneously.

public class History
{
  // thread-safety issues !!!! 
  // In fact, here you should use a StringBuffer or some locking.
  private StringBuilder historyBuilder = new StringBuilder();

  public void saveEvent(String event)
  {
     historyBuilder.append(event).append('\n');
  }

  public String getHistoryString()
  {
     return historyBuilder.toString();
  }
}

But compiler optimizations will not create these kind of constructions. The StringBuilder is always created and used only within one and the same thread.

We could try to make things more complex (static fields, multiple classloaders, ...) but always again, each StringBuilder instance is created and used by only 1 thread.

EDIT:

Perhaps useful to know: This optimization happens during the generation of the byte-code. There are other optimizations later on during JIT compilation, but this optimization is not one of them. However the JIT compiler does have an important impact in the final performance.

Upvotes: 1

Sotirios Delimanolis
Sotirios Delimanolis

Reputation: 280102

The Java Language Specification states

The result of string concatenation is a reference to a String object that is the concatenation of the two operand strings. The characters of the left-hand operand precede the characters of the right-hand operand in the newly created string.

So, although a compiler is free to optimize how the concatenation happens, it must do so while following that rule, "a" + "b" becomes "ab". In an unthread-safe, shared StringBuilder, implementation that would potentially not be the case. That implementation would therefore not be correct and could not be considered Java.

Upvotes: 1

Mateusz Dymczyk
Mateusz Dymczyk

Reputation: 15141

No, there is no state that's being shared between different threads so the situation you described can not happen.

If instead st was a member variable of that class, instead of being passed as an argument, and was incremented - that's a different story.

How it works now is that st will be put on the execution stack, each thread has it's own execution stack and they don't share stuff from there. Therefore each thread has it's own value of st. When it's a member variable of a class it's in memory (single value) and all threads would try to use it (the same one).

@Edit: well I guess it is possible if you call the method several times with the same value :-))

Upvotes: 11

Smutje
Smutje

Reputation: 18163

As you don't share any field between your threads, the order of printing can differ, but any problem concerning thread safety (race conditions) shouldn't appear.

Upvotes: 2

TheLostMind
TheLostMind

Reputation: 36304

Could there be instances such that it could print the following i.e. some thread numbers are lost and some numbers are doubled?

st is a method local variable, also st doesn't escape the method's scope so it is thread-safe. So, multithreading will have no effect on st . The messages can be printed out of order depending on which thread runs the method at what time.

Upvotes: 5

Related Questions