Would it be good to check if there's an interned "version" of a string before adding it to the StringPool?

[phizch]

I just "relearned" about string interning, and I also now had a look at the StringPool implementation, and noticed that there's not check if a string has an "interned version" before adding it to the pool. Could this be a way of saving some memory, and possibly increase performance for some operations?

Note; I'm not suggesting to actually intern strings, just check if they already are, and if so use that reference.

dotnet/src/CommunityToolkit.HighPerformance/Buffers/StringPool.cs

Lines 433 to 447 in 0a2e8ed

	public string GetOrAdd(ReadOnlySpan<char> span, int hashcode)
	{
	ref string result = ref TryGet(span, hashcode);
	if (!Unsafe.IsNullRef(ref result))
	{
	return result;
	}
	string value = span.ToString();
	Insert(value, hashcode);
	return value;
	}

e.g.

@ line 443:
var interned = string.IsInterned( value );
if( interned is not null ) value = interned;

Of course, the same should be done for the other Add/GetOrAdd methods too.

The main reason I'm asking is that I currently have a static constructor in one of my classes that does the following for all strings expected by my parser:

StringPool.Shared.Add( "..." )

Since the strings added by the constructor are literals, they would have been interned by the compiler(/JIT?), and a StringPool would find and use the interned version, even after a Reset, or when a new StringPool is used.

From what I understand about String.IsInterned:

var myString = "My String";
var my = Regex.Match( myString, "My" ).Value;

Debug.Assert( ReferenceEquals( my, "My" ) is false );

var fromSp = StringPool.Shared.GetOrAdd( my );
Debug.Assert( ReferenceEquals( fromSp, "My" ) is false );

// "My" is interned since it's used as a string literal in the code.
var interned = string.IsInterned( my ); 
Debug.Assert( interned is not null );
Debug.Assert( ReferenceEquals( interned, "My" ) );

// update the value in the string pool with the interned version
StringPool.Shared.Add( interned );

// get the updated string from the pool
var fromSp2 = StringPool.Shared.GetOrAdd( my );

// now the value gotten from the pool is interned
Debug.Assert( ReferenceEquals( fromSp2, "My" ));

The first step of String.Equals is to do a ReferenceEquals, so there should be some performance gains where equality is compared. I'm not 100% sure, but e.g. switch statements.

[john-h-k]

Checking string intern is actually a deceivingly expensive operation. The idea of StringPool is that if you are using it, then any strings which would be interned are already in the pool and so interning just means double-checking every string added/retrieved

[phizch]

Oops..
Tbh. I expected IsInterned to be really fast.. Maybe I should have checked that first 😅. Anyway, It might still be useful for "preloading" the pool at startup.. Of course, if that's done using literal strings, they would be interned in the first place..