Regarding String.replace

[vjay82]

Hi,

as a follow up on #10170 (comment) I experimented a bit:

https://jsbenchmark.com/#eyJjYXNlcyI6W3siaWQiOiJWTXJCVVlXWW85SUZnb3l2Q3d2OUIiLCJjb2RlIjoidmFyIGEgPSBcInRoaXMgaXMgYSB0ZXN0IHN0cmluZ1wiICsgTWF0aC5yYW5kb20oKTsgdmFyIGIgPSBcImFhJGJiXCIgKyBNYXRoLnJhbmRvbSgpOyBhLnJlcGxhY2VBbGwoXCJpXCIsIGIucmVwbGFjZUFsbChcIiRcIiwgXCIkJCQkXCIpKSIsIm5hbWUiOiJ0d2ljZSB3aXRoIGRvbGxhciIsImRlcGVuZGVuY2llcyI6W119LHsiaWQiOiJIYV9wUjZNLU1uOGpRbG9uQmZRcmQiLCJjb2RlIjoidmFyIGEgPSBcInRoaXMgaXMgYSB0ZXN0IHN0cmluZ1wiICsgTWF0aC5yYW5kb20oKTsgdmFyIGIgPSBcImFhc2JiXCIgKyBNYXRoLnJhbmRvbSgpOyBhLnJlcGxhY2VBbGwoXCJpXCIsIGIucmVwbGFjZUFsbChcIiRcIiwgXCIkJCQkXCIpKSIsIm5hbWUiOiJ0d2ljZSB3aXRob3V0IGRvbGxhciIsImRlcGVuZGVuY2llcyI6W119LHsiaWQiOiJ2UTAzdVA5Vy1DYWptOHVXdUhOX24iLCJjb2RlIjoidmFyIGEgPSBcInRoaXMgaXMgYSB0ZXN0IHN0cmluZ1wiICsgTWF0aC5yYW5kb20oKTsgdmFyIGIgPSBcImFhc2JiXCIgKyBNYXRoLnJhbmRvbSgpOyAgYS5yZXBsYWNlQWxsKFwiaVwiLCBiKSIsIm5hbWUiOiJmYXN0UGF0aCIsImRlcGVuZGVuY2llcyI6W119LHsiaWQiOiJOb0lPNVV5NDFucnVWNThCUk9HVk8iLCJjb2RlIjoidmFyIGEgPSBcInRoaXMgaXMgYSB0ZXN0IHN0cmluZ1wiICsgTWF0aC5yYW5kb20oKTsgdmFyIGIgPSBcImFhJGJiXCIgKyBNYXRoLnJhbmRvbSgpOyBhLnJlcGxhY2VBbGwoXCJpXCIsIGZ1bmN0aW9uKCkge3JldHVybiBifSkiLCJuYW1lIjoiZnVuY3Rpb24iLCJkZXBlbmRlbmNpZXMiOltdfSx7ImlkIjoianlTWElWR21rdldEVzZDNlFJVXBwIiwiY29kZSI6InZhciBhID0gXCJ0aGlzIGlzIGEgdGVzdCBzdHJpbmdcIiArIE1hdGgucmFuZG9tKCk7IHZhciBiID0gXCJhYSRiYlwiICsgTWF0aC5yYW5kb20oKTsgYS5yZXBsYWNlQWxsKFwiaVwiLCAoKSA9PiB7cmV0dXJuIGJ9KSIsIm5hbWUiOiJsYW1iZGEiLCJkZXBlbmRlbmNpZXMiOltdfV0sImNvbmZpZyI6eyJuYW1lIjoiQmFzaWMgZXhhbXBsZSIsInBhcmFsbGVsIjp0cnVlLCJnbG9iYWxUZXN0Q29uZmlnIjp7ImRlcGVuZGVuY2llcyI6W119LCJkYXRhQ29kZSI6IiJ9fQ

On my machine "function"/"lambda" seems to be the overall winner.
("fastPath" wins in Firefox in case of constant input strings but not in Chrome.)

So my proposal would be something like this:

	public String replace(CharSequence from, CharSequence to) {
		return asNativeString().replaceAll(from.toString(), () -> {
			return to.toString();
		});
	}

Regards

[niloc132]

Lambdas generate quite a bit of code (a new type token, class setup, constructor to assign fields for closed variables, and makeLambda() invocation at the callsite for jsfunctions).

I think we should compare finished cases that pass tests so that we're measuring the flexible parts rather than just the mechanics of calling the function natively - this overhead might turn out to just be noise (or not) when we get the rest of the replacement correct.

[vjay82]

So what you are saying is, a native function would be lighter than using the following?

@JsFunction
public interface StringProducer {
	public String produce();
}

And that would probably also apply to runtime behavior (because of clinit)?

Then?

	public String replace(char from, char to) {
		return replace0(this, String.valueOf(from), String.valueOf(to));
	}

	public String replace(CharSequence from, CharSequence to) {
		return replace0(this, from.toString(), to.toString());
	}
	
	private static native String replace0(String s, String from, String to) /*-{
		return s.replaceAll(from, function() {return to});
	}-*/;

[niloc132]

Without measuring I can't be sure, I'm only making the note (for your own proj, for anyone else who tries to implement this, etc) that measuring is a good idea to see how it ends up in the output, so that we can be confident that we aren't adding extra expense to every GWT app out there.

Not following your clinit question - String doesn't have a clinit today, and won't get one from your changes, unless I misunderstand?

[vjay82]

The Lambda (if done in Java) could get one (clinit) currently, right? Not sure if the annotation prevents that - probably not. Yes, I agree, will check if having time at hand again. Also the general performance improvement (which is sadly not possible to measure in a JS sandbox website) would be interesting. The benchmark, I did so far, shows that the function-approach outperforms even calling replaceAll 2 times. I deem it very likely that the current (GWT) implementation doesn't hold a candle against that. Will provide a benchmark when having some free time.

[niloc132]

By themselves, lambdas shouldn't have a clinit - though as a non-static inner class, if they are inlined into an external class, they could end up with a clinit for the outer class if one exists.

It looks like I misremembered: String will have to have a clinit for CASE_INSENSITIVE_ORDER, unless it is compiled out. Briefly reviewing some compiled GWT apps, that field is never used within GWT itself and rarely/ever used in other apps, and it compiles out cleanly, so no clinit should be emitted at all most of the time. (Eagerly initing String seems like an easy win, I know you're tracking the issue for that - better handling of inner classes calling the outer class clinit is tracked by #10159.)

[vjay82]

Yes, currently overriding String and other classes to eagerly initiate them in onModuleLoad() to get rid of that overhead (am actually one of the users of CASE_INSENSITIVE_ORDER).
Only downside is, that it feels (and probably is) really dirty.

[vjay82]

Hi,

I had the time to write a benchmark. It is attached as the last listing. It can be run via

new StringReplaceTest().init();

from any GWT application.

The following approaches are measured against each other:

1. The String.replace function, GWT brings.
2. Calling the JavaScript String.replaceAll function via jsinterop and passing a Java lambda function as the second parameter.
3. Using JSNI to call JavaScript's String.replaceAll and passing a JS function as the second parameter.
4. Using JSNI to call JavaScript's String.replaceAll twice to properly escape "$".

The results on my machine (compiled with all flags towards performance):

Totals after 1000 iterations:

1. String.replace took: 1146 ms
2. Lambda took: 388 ms
3. JSNI function took: 370 ms
4. JSNI twice twice took: 950 ms

Running without optimizations on the CodeServer, the numbers don't look very different.

I was interested, why approach 2 (lambda) is slower than approach 3 (pure JSNI).
Looking at the code (compiled with style=DETAILED), one can see, that there are some extra steps involved:

  var replaceWith, s;
  s = $intern_111 + $wnd.Math.random();
  replaceWith = $intern_112 + $wnd.Math.random();
  s.replaceAll('o', com_google_gwt_lang_Runtime_makeLambdaFunction__Lcom_google_gwt_core_client_JavaScriptObject_2Lcom_google_gwt_core_client_JavaScriptObject_2Lcom_google_gwt_core_client_JavaScriptObject_2Lcom_google_gwt_core_client_JavaScriptObject_2(de_wtk_client_StringReplaceTest$lambda$0$Type_StringReplaceTest$lambda$0$Type__Ljava_lang_CharSequence_2V.prototype.produce__Ljava_lang_String_2, de_wtk_client_StringReplaceTest$lambda$0$Type_StringReplaceTest$lambda$0$Type__Ljava_lang_CharSequence_2V, [replaceWith]));

The description for makeLambdaFunction reads:

Create a function that applies the specified samMethod on itself, and whose proto points to instance.

Which probably explains the small overhead.

On my fork of GWT I switched to approach 3 (which probably also reduces code size a bit).
Posting this here to contribute something back to the project, allowing you to make an informed decision in which direction you like to move (or not move at all).

Following the StringReplaceTest file, if you like to give it a quick benchmark yourself:

package YOUR.PACKAGE;

import com.google.gwt.user.client.Timer;

import javaemul.internal.JsUtils;
import jsinterop.annotations.JsFunction;
import jsinterop.annotations.JsType;

public class StringReplaceTest {

	private static final int ITERATION_COUNT = 10;

	@JsFunction
	public static interface StringProducer {
		public String produce();
	}

	@JsType(isNative = true, name = "String", namespace = "<window>")
	private static class MyNativeString {
		public native String replaceAll(String target, StringProducer replaceWith);
	}

	/**
	 * Bypass our code optimizer.
	 */
	private static native void cout(String s) /*-{
		console.log(s);
	}-*/;

	private static native String replaceJniFunction(String s, String from, String to) /*-{
		return s.replaceAll(from, function() {return to});
	}-*/;

	private static native String replaceJniTwiceTwice(String s, String from, String to) /*-{
		return s.replaceAll(from, to.replace("$", "$$$$"));
	}-*/;

	private static MyNativeString asNativeString(String s) {
		return JsUtils.uncheckedCast(s);
	}

	public static String replaceLambda(String s, CharSequence from, CharSequence to) {
		return asNativeString(s).replaceAll(from.toString(), () -> {
			return to.toString();
		});
	}

	public static long measure(int cycles, Runnable r) {
		for (int i = 0; i < 10; i++) {
			r.run();
		}
		long ts = System.currentTimeMillis();
		for (int i = 0; i < cycles; i++) {
			r.run();
		}
		return System.currentTimeMillis() - ts;
	}

	private Timer t;
	private long runCount = 0;
	private long stringReplaceTotalTime = 0;
	private long lambdaTotalTime = 0;
	private long jsniFunctionTotalTime = 0;
	private long jsniTwiceTwiceTotalTime = 0;

	/**
	 * Calls test() every second to give browser some time to breathe.  
	 */
	public void init() {
		cout("Starting performance measurement...");

		t = new Timer() {

			@Override
			public void run() {
				test();
			}

		};
		t.scheduleRepeating(1000);
	}

	private void test() {

		runCount++;

		long timeTaken = measure(1_000_000, new Runnable() {
			@Override
			public void run() {
				String s = "The quick brown fox jumps over the lazy dog." + Math.random();
				String replaceWith = "test$" + Math.random();
				s.replace("o", replaceWith);
			}
		});
		stringReplaceTotalTime += timeTaken;
		cout("String.replace took: " + timeTaken + " (avg: " + (stringReplaceTotalTime / runCount) + ")");

		timeTaken = measure(1_000_000, new Runnable() {
			@Override
			public void run() {
				String s = "The quick brown fox jumps over the lazy dog." + Math.random();
				String replaceWith = "test$" + Math.random();
				replaceLambda(s, "o", replaceWith);
			}
		});
		lambdaTotalTime += timeTaken;
		cout("Lambda took: " + timeTaken + " (avg: " + (lambdaTotalTime / runCount) + ")");

		timeTaken = measure(1_000_000, new Runnable() {
			@Override
			public void run() {
				String s = "The quick brown fox jumps over the lazy dog." + Math.random();
				String replaceWith = "test$" + Math.random();
				replaceJniFunction(s, "o", replaceWith);
			}
		});
		jsniFunctionTotalTime += timeTaken;
		cout("JSNI function took: " + timeTaken + " (avg: " + (jsniFunctionTotalTime / runCount) + ")");

		timeTaken = measure(1_000_000, new Runnable() {
			@Override
			public void run() {
				String s = "The quick brown fox jumps over the lazy dog." + Math.random();
				String replaceWith = "test$" + Math.random();
				replaceJniTwiceTwice(s, "o", replaceWith);
			}
		});
		jsniTwiceTwiceTotalTime += timeTaken;
		cout("JSNI twice twice took: " + timeTaken + " (avg: " + (jsniTwiceTwiceTotalTime / runCount) + ")");

		if (runCount == ITERATION_COUNT) {
			t.cancel();
			cout("Totals after " + ITERATION_COUNT + " iterations:");
			cout("String.replace took: " + (stringReplaceTotalTime / runCount) + " ms");
			cout("Lambda took: " + (lambdaTotalTime / runCount) + " ms");
			cout("JSNI function took: " + (jsniFunctionTotalTime / runCount) + " ms");
			cout("JSNI twice twice took: " + (jsniTwiceTwiceTotalTime / runCount) + " ms");
		}
	}

}

[niloc132]

	public static String replaceLambda(String s, CharSequence from, CharSequence to) {
		return asNativeString(s).replaceAll(from.toString(), () -> {
			return to.toString();
		});
	}

  s.replaceAll('o', com_google_gwt_lang_Runtime_makeLambdaFunction__Lcom_google_gwt_core_client_JavaScriptObject_2Lcom_google_gwt_core_client_JavaScriptObject_2Lcom_google_gwt_core_client_JavaScriptObject_2Lcom_google_gwt_core_client_JavaScriptObject_2(de_wtk_client_StringReplaceTest$lambda$0$Type_StringReplaceTest$lambda$0$Type__Ljava_lang_CharSequence_2V.prototype.produce__Ljava_lang_String_2, de_wtk_client_StringReplaceTest$lambda$0$Type_StringReplaceTest$lambda$0$Type__Ljava_lang_CharSequence_2V, [replaceWith]));

I'm not sure we have an easy "makes sense in Java, translates to JS that can be used easily" approach here without fundamentally rewriting how lambdas and method references work (in fairness, I think it was understood at the time that this could be done more like how the JVM does it, but the approach we have from 2.8 onward does a pretty good job most of the time). It mostly boils down to the fact that functions are first class in JS, but not in Java, so we have to have a proper class-looking thing (with a constructor and field to close over the outer variables, a getClass() and class literal which could be compiled away, etc), then we need to pass only the SAM method out to JS (which still needs to reference the Java instance, the proto you noted.

There's no technical reason why we couldn't have an approach where the lambda becomes an inline function when we are certain it can't escape in some way you could call .getClass() or whatever on it - no reason except for the time to rewrite the appropriate parts of the compiler.

Aside: call toString() on to before passing it in for a very tiny improvement - your JSNI is getting that "for free" since you declare it as a string instead of charseq. Odds are, the compiler notices that you are calling the replaceLambda with Strings anyway and tightens up from CharSeq to String, and so rewrites away the actual toString call.

As another way around, you could just make a function out of plain js that you pass around in java - consider func = "".toString as the function, and passing it to the JS replaceAll as func.bind(to.toString()). That is now a js lambda that returns to.toString() when is called, with you paying nothing for the lambda:

import elemental2.core.Function;

//...

	@JsType(isNative = true, name = "String", namespace = "<window>")
	private static class MyNativeString {
		public native String replaceAll(String target, Function replaceWith);
	}

	public static String replaceLambda(String s, CharSequence from, CharSequence to) {
		String replacement = to.toString();
		// This could be a constant
		Function f = Js.asPropertyMap("").getAsAny("toString").cast();
		return asNativeString(s).replaceAll(from.toString(), f.bind(replacement));
	}

There's probably a more concise way to do this, but if we're golfing, that's the direction I'd take it.

[vjay82]

That compiler improvement would be really nice but I also understand that it is no small undertaking.

Regarding toString - I used CharSequences to stay compatible with the method signature of String.replace - as far as I have seen, the compile does away with it anyway.

You like me to give your proposal a spin? However, it will probably play in the ballpark of approach 2/3.
In my book all of these are okay (and better than how things are today).

Do you like me to create a pull request or prefer doing it yourself (maybe faster than playing ping pong)?

If not interested at all, it's okay to close this. It is just a proposal.

[niloc132]

Regarding toString - I used CharSequences to stay compatible with the method signature of String.replace - as far as I have seen, the compile does away with it anyway.

Yes, write the code for CharSequence, I'm just suggesting that if you do transform to string, do it sooner than in the lambda. Type tightening will catch it, unless you are passing other CS instances like StringBuilder, then it will have to leave it as CharSequence - unless it can inline the usages away so that each is appropriately specialized.

You like me to give your proposal a spin? However, it will probably play in the ballpark of approach 2/3.

I'd love a patch based on the work you've done, without or without my suggestions - I'm only offering them to make conversation, see if it gives you any better ideas along the way. If the compiled size is the same or better and performance is better, I think it will be an easy win. Existing tests are probably enough, the impls all look pretty simple so we shouldn't have more edge cases to worry about. We don't have a good set of metrics for checking size for representative apps - you can grep the build for compiled size of all samples is (part of the output of ant samples) to see what the new size is for our samples - but this is permutation and size weighted (an app with many permutations will dominate results, large apps will dominate results, etc).

build:
     [echo] compiled size of all samples is 3338616 bytes.

[vjay82]

I updated the benchmark by your proposal in two versions.

1. The String.replace function, GWT brings.
2. Calling the JavaScript String.replaceAll function via jsinterop and passing a Java lambda function as the second parameter.
3. Using JSNI to call JavaScript's String.replaceAll and passing a JS function as the second parameter.
4. Using JSNI to call JavaScript's String.replaceAll twice to properly escape "$".
   --
5. Your approach as stated.
6. Your apporach but made the function f a static field.

The results are as following:

Totals after 200 iterations:
String.replace took: 1194 ms
Lambda took: 408 ms
JSNI function took: 386 ms
JSNI twice twice took: 998 ms
Lambda Niloc took: 380 ms
Lambda Niloc static took: 382 ms

So your proposal offers the same performance as approach 3 does. What surprises me, is, that static initialization (6) adds nothing. It may result in smaller size as "bind" might be shorter when inlining often. However, as long as early clinit is not implemented, we might shoot ourselves in the foot by introducing String.clinit calls (or we need some kind of if ... initializer, which would blow things up too).

My personal favorite is still approach 3 as it is so much less code and complexity. I don't even understand what "Function f = Js.asPropertyMap("").getAsAny("toString").cast();" does. Anyway, I can create a pull request implementing approach 5. There is just one little issue: It relies on elemental2.core. For this benchmark I plucked the Function class out of it. I don't think it would make a good addition to the String class, right?

Regarding type tightening: With so much magic behind the scenes (GWT and V8 probably has a lot of fast-paths too), it is hard to guess, which code compiles to the best/fastest result at the end of the day.

Updated Testclass:

package YOUR.PACKAGE;

import com.google.gwt.user.client.Timer;

import javaemul.internal.JsUtils;
import jsinterop.annotations.JsFunction;
import jsinterop.annotations.JsMethod;
import jsinterop.annotations.JsPackage;
import jsinterop.annotations.JsType;
import jsinterop.base.Js;

public class StringReplaceTest {

	private static final int ITERATION_COUNT = 200;

	@JsFunction
	public static interface StringProducer {
		public String produce();
	}

	@JsType(isNative = true, namespace = JsPackage.GLOBAL)
	public static class Function {
		public Object arguments;
		@Deprecated
		public double arity;
		public Function caller;
		public Object displayName;
		public int length;
		public String name;

		public Function(Object... var_args) {
		}

		public native Object apply(Object... var_args);

		public native Function bind(Object selfObj, Object... var_args);

		public native Object call(Object... var_args);

		@JsMethod(name = "toString")
		public native String toString_();
	}

	@JsType(isNative = true, name = "String", namespace = "<window>")
	private static class MyNativeString {
		public native String replaceAll(String target, StringProducer replaceWith);
		public native String replaceAll(String target, Function replaceWith);
	}

	/**
	 * Bypass our code optimizer.
	 */
	private static native void cout(String s) /*-{
		console.log(s);
	}-*/;

	private static native String replaceJniFunction(String s, String from, String to) /*-{
		return s.replaceAll(from, function() {return to});
	}-*/;

	private static native String replaceJniTwiceTwice(String s, String from, String to) /*-{
		return s.replaceAll(from, to.replace("$", "$$$$"));
	}-*/;

	private static MyNativeString asNativeString(String s) {
		return JsUtils.uncheckedCast(s);
	}

	public static String replaceLambda(String s, CharSequence from, CharSequence to) {
		return asNativeString(s).replaceAll(from.toString(), () -> {
			return to.toString();
		});
	}

	public static String replaceLambdaNiloc(String s, CharSequence from, CharSequence to) {
		String replacement = to.toString();
		// This could be a constant
		Function f = Js.asPropertyMap("").getAsAny("toString").cast();
		return asNativeString(s).replaceAll(from.toString(), f.bind(replacement));
	}

	private static Function toStringFunction = Js.asPropertyMap("").getAsAny("toString").cast();
	public static String replaceLambdaNilocStatic(String s, CharSequence from, CharSequence to) {
		String replacement = to.toString();
		return asNativeString(s).replaceAll(from.toString(), toStringFunction.bind(replacement));
	}

	public static long measure(int cycles, Runnable r) {
		for (int i = 0; i < 10; i++) {
			r.run();
		}
		long ts = System.currentTimeMillis();
		for (int i = 0; i < cycles; i++) {
			r.run();
		}
		return System.currentTimeMillis() - ts;
	}

	private Timer t;
	private long runCount = 0;
	private long stringReplaceTotalTime = 0;
	private long lambdaTotalTime = 0;
	private long lambdaNilocTotalTime = 0;
	private long lambdaNilocStaticTotalTime = 0;
	private long jsniFunctionTotalTime = 0;
	private long jsniTwiceTwiceTotalTime = 0;

	/**
	 * Calls test() every second to give browser some time to breathe.  
	 */
	public void init() {
		cout("Starting performance measurement...");

		t = new Timer() {

			@Override
			public void run() {
				test();
			}

		};
		t.scheduleRepeating(1000);
	}

	private void test() {

		runCount++;

		long timeTaken = measure(1_000_000, () -> {
			String s = "The quick brown fox jumps over the lazy dog." + Math.random();
			String replaceWith = "test$" + Math.random();
			s.replace("o", replaceWith);
		});
		stringReplaceTotalTime += timeTaken;
		cout("String.replace took: " + timeTaken + " (avg: " + (stringReplaceTotalTime / runCount) + ")");

		timeTaken = measure(1_000_000, () -> {
			String s = "The quick brown fox jumps over the lazy dog." + Math.random();
			String replaceWith = "test$" + Math.random();
			replaceLambda(s, "o", replaceWith);
		});
		lambdaTotalTime += timeTaken;
		cout("Lambda took: " + timeTaken + " (avg: " + (lambdaTotalTime / runCount) + ")");

		timeTaken = measure(1_000_000, () -> {
			String s = "The quick brown fox jumps over the lazy dog." + Math.random();
			String replaceWith = "test$" + Math.random();
			replaceJniFunction(s, "o", replaceWith);
		});
		jsniFunctionTotalTime += timeTaken;
		cout("JSNI function took: " + timeTaken + " (avg: " + (jsniFunctionTotalTime / runCount) + ")");

		timeTaken = measure(1_000_000, () -> {
			String s = "The quick brown fox jumps over the lazy dog." + Math.random();
			String replaceWith = "test$" + Math.random();
			replaceJniTwiceTwice(s, "o", replaceWith);
		});
		jsniTwiceTwiceTotalTime += timeTaken;
		cout("JSNI twice twice took: " + timeTaken + " (avg: " + (jsniTwiceTwiceTotalTime / runCount) + ")");

		timeTaken = measure(1_000_000, () -> {
			String s = "The quick brown fox jumps over the lazy dog." + Math.random();
			String replaceWith = "test$" + Math.random();
			replaceLambdaNiloc(s, "o", replaceWith);
		});
		lambdaNilocTotalTime += timeTaken;
		cout("Lambda Niloc took: " + timeTaken + " (avg: " + (lambdaNilocTotalTime / runCount) + ")");

		timeTaken = measure(1_000_000, () -> {
			String s = "The quick brown fox jumps over the lazy dog." + Math.random();
			String replaceWith = "test$" + Math.random();
			replaceLambdaNilocStatic(s, "o", replaceWith);
		});
		lambdaNilocStaticTotalTime += timeTaken;
		cout("Lambda Niloc static took: " + timeTaken + " (avg: " + (lambdaNilocStaticTotalTime / runCount) + ")");

		if (runCount == ITERATION_COUNT) {
			t.cancel();
			cout("Totals after " + ITERATION_COUNT + " iterations:");
			cout("String.replace took: " + (stringReplaceTotalTime / runCount) + " ms");
			cout("Lambda took: " + (lambdaTotalTime / runCount) + " ms");
			cout("JSNI function took: " + (jsniFunctionTotalTime / runCount) + " ms");
			cout("JSNI twice twice took: " + (jsniTwiceTwiceTotalTime / runCount) + " ms");
			cout("Lambda Niloc took: " + (lambdaNilocTotalTime / runCount) + " ms");
			cout("Lambda Niloc static took: " + (lambdaNilocStaticTotalTime / runCount) + " ms");
		}
	}

}

[niloc132]

I don't even understand what "Function f = Js.asPropertyMap("").getAsAny("toString").cast();" does. Anyway, I can create a pull request implementing approach 5.

I'm sorry, I should have been clearer - the JsInterop junk is just to do what would be obvious (to me) in JS, grabbing a reference to the String.prototype.toString function (except just "".toString by doing it that way), and bind() is a way to give it a "this" so that you don't need to invoke it directly. This means that these two techniques are effectively the same:

var object = ...
return function() {
  return object.foo();
};

return object.foo.bind(object);

Since making the closure adds a small expense for GWT code, taking the other approach is slightly preferred.

The extra clinit call also occurred to me, and I'd tend to say avoid it, at least until we can make it free. No promises on a deadline there.

There is just one little issue: It relies on elemental2.core. For this benchmark I plucked the Function class out of it. I don't think it would make a good addition to the String class, right?

No elemental or jsinterop-base is required - we can write a bit of jsinterop and stuff it in String or in JsUtils with all there rest, or just write some JSNI. Broadly speaking JSNI is worse choice in terms of "can the compiler improve this in some way" or "how easy will this be to maintain", but it certainly has its place. I'd remove everything in Function except the bind() method.

Regarding type tightening: With so much magic behind the scenes (GWT and V8 probably has a lot of fast-paths too), it is hard to guess, which code compiles to the best/fastest result at the end of the day.

Totally agreed - light testing, a little reading of conventional wisdom, and a dash of intuition is really all I do for runtime perf, but compile size is easier to measure. Plus a few easy ones, like "broadly, code you don't execute will be faster". I would guess that the 1-2% improvement from not using the static field is probably just noise.

[vjay82]

Following the sizes:

Original:
compiled size of all samples is 3320023 bytes.

JSNI:
compiled size of all samples is 3315743 bytes.

    public String replace(char from, char to) {
		return replace0(this, String.valueOf(from), String.valueOf(to));
	}

	public String replace(CharSequence from, CharSequence to) {
		return replace0(this, from.toString(), to.toString());
	}
	
	private static native String replace0(String s, String from, String to) /*-{
		return s.replaceAll(from, function() {return to});
	}-*/;

NILOC:
compiled size of all samples is 3315863 bytes.
For this I modified the proposed solution to not use jsinterop.base

First, I extended the already existing NativeFunction:

  @JsType(isNative = true, name = "Function", namespace = JsPackage.GLOBAL)
  private static class NativeFunction {
    public native String apply(String thisContext, Object[] argsArray);
	public native NativeFunction bind(Object selfObj, Object... var_args);
  }

Then substituted jsinsterop.base with JSUtils:

  public String replace(char from, char to) {
    return replace0(String.valueOf(from), String.valueOf(to));
  }

  public String replace(CharSequence from, CharSequence to) {
    return replace0(from.toString(), to.toString());
  }
  
  public String replace0(String from, String to) {
    NativeFunction f = JsUtils.getProperty("", "toString");
	return asNativeString().replaceAll(from, f.bind(to));
  }

And finally extended NativeString:

  @JsType(isNative = true, name = "String", namespace = "<window>")
  private static class NativeString {
    ...
	public native String replaceAll(String target, NativeFunction replaceWith);
  }

For me JSNI looks like the simplest and smallest solution (and the one I will use).
However, as you prefer jsinterop, should I send a PR with that? Any modifications?

[niloc132]

I think JSNI is probably best here, though if we're optimizing for size, I might try to "golf" it a bit more, and see what we can do. Write the PR with JSNI though, and if I get too picky, it becomes my problem. Briefly, the one I can see is to replace function(){return to} (note that you can add the semi where it belongs, the compiler will remove it) with the bind call, to.toString.bind(to) and save a very bytes (note replacing "" with to, which I now realize will be a byte shorter in OBF because the argument to will almost always be b or c).

Historical note: there is no other JSNI in String at this time due to the J2CL/GWT split. Google rewrote all JSNI out of the main emul classes so that j2cl could share the entire library between the two compilers - then, then decided to "hard fork" the rest of the way, so that j2cl had its own copies. That has since continued - even within j2cl, there are two String.java classes (j2wasm and j2cl), both retaining TODOs from the original GWT source but also specific references in comments to how the GWT compiler works. This means we can safely add some back judiciously - JSNI doesn't optimize as well as Java - this is a case where almost nothing is happening except interfacing with the JS runtime, so it seems okay.

[vjay82]

Ok, will do.

Historical note: That seems to be quite messy.