Skip to content

Commit

Permalink
Use Dictionary for underlying cache of ResourceSet (#44104)
Browse files Browse the repository at this point in the history
  • Loading branch information
@marek-safar
marek-safar committed Nov 6, 2020
1 parent 54d27b2 commit a683543
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 64 deletions.
107 changes: 43 additions & 64 deletions src/libraries/System.Private.CoreLib/src/System/Resources/ResourceSet.cs
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

/*============================================================
**
**
**
**
**
** Purpose: Culture-specific collection of resources.
**
**
===========================================================*/

using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
Expand All @@ -29,15 +19,15 @@ namespace System.Resources
public class ResourceSet : IDisposable, IEnumerable
{
protected IResourceReader Reader = null!;
internal Hashtable? Table;

private Hashtable? _caseInsensitiveTable; // For case-insensitive lookups.
private Dictionary<object, object?>? _table;
private Dictionary<string, object?>? _caseInsensitiveTable; // For case-insensitive lookups.

protected ResourceSet()
{
// To not inconvenience people subclassing us, we should allocate a new
// hashtable here just so that Table is set to something.
Table = new Hashtable();
_table = new Dictionary<object, object?>();
}

// For RuntimeResourceSet, ignore the Table parameter - it's a wasted
Expand Down Expand Up @@ -96,7 +86,7 @@ protected virtual void Dispose(bool disposing)
}
Reader = null!;
_caseInsensitiveTable = null;
Table = null;
_table = null;
}

public void Dispose()
Expand Down Expand Up @@ -134,59 +124,46 @@ IEnumerator IEnumerable.GetEnumerator()

private IDictionaryEnumerator GetEnumeratorHelper()
{
Hashtable? copyOfTable = Table; // Avoid a race with Dispose
if (copyOfTable == null)
throw new ObjectDisposedException(null, SR.ObjectDisposed_ResourceSet);
return copyOfTable.GetEnumerator();
// Avoid a race with Dispose
return _table?.GetEnumerator() ?? throw new ObjectDisposedException(null, SR.ObjectDisposed_ResourceSet);
}

// Look up a string value for a resource given its name.
//
public virtual string? GetString(string name)
{
object? obj = GetObjectInternal(name);
try
{
return (string?)obj;
}
catch (InvalidCastException)
{
throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Name, name));
}
if (obj is string s)
return s;

if (obj is null)
return null;

throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Name, name));
}

public virtual string? GetString(string name, bool ignoreCase)
{
object? obj;
string? s;

// Case-sensitive lookup
obj = GetObjectInternal(name);
try
{
s = (string?)obj;
}
catch (InvalidCastException)
{
object? obj = GetObjectInternal(name);
if (obj is string s)
return s;

if (obj is not null)
throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Name, name));
}

// case-sensitive lookup succeeded
if (s != null || !ignoreCase)
{
return s;
}
if (!ignoreCase)
return null;

// Try doing a case-insensitive lookup
obj = GetCaseInsensitiveObjectInternal(name);
try
{
return (string?)obj;
}
catch (InvalidCastException)
{
throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Name, name));
}
if (obj is string si)
return si;

if (obj is null)
return null;

throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Name, name));
}

// Look up an object value for a resource given its name.
Expand All @@ -208,13 +185,12 @@ private IDictionaryEnumerator GetEnumeratorHelper()

protected virtual void ReadResources()
{
Debug.Assert(Table != null);
Debug.Assert(_table != null);
Debug.Assert(Reader != null);
IDictionaryEnumerator en = Reader.GetEnumerator();
while (en.MoveNext())
{
object? value = en.Value;
Table.Add(en.Key, value);
_table.Add(en.Key, en.Value);
}
// While technically possible to close the Reader here, don't close it
// to help with some WinRes lifetime issues.
Expand All @@ -225,35 +201,38 @@ protected virtual void ReadResources()
if (name == null)
throw new ArgumentNullException(nameof(name));

Hashtable? copyOfTable = Table; // Avoid a race with Dispose
Dictionary<object, object?>? copyOfTable = _table; // Avoid a race with Dispose

if (copyOfTable == null)
throw new ObjectDisposedException(null, SR.ObjectDisposed_ResourceSet);

return copyOfTable[name];
copyOfTable.TryGetValue(name, out object? value);
return value;
}

private object? GetCaseInsensitiveObjectInternal(string name)
{
Hashtable? copyOfTable = Table; // Avoid a race with Dispose
Dictionary<object, object?>? copyOfTable = _table; // Avoid a race with Dispose

if (copyOfTable == null)
throw new ObjectDisposedException(null, SR.ObjectDisposed_ResourceSet);

Hashtable? caseTable = _caseInsensitiveTable; // Avoid a race condition with Close
Dictionary<string, object?>? caseTable = _caseInsensitiveTable; // Avoid a race condition with Close
if (caseTable == null)
{
caseTable = new Hashtable(StringComparer.OrdinalIgnoreCase);

IDictionaryEnumerator en = copyOfTable.GetEnumerator();
while (en.MoveNext())
caseTable = new Dictionary<string, object?>(copyOfTable.Count, StringComparer.OrdinalIgnoreCase);
foreach (var item in copyOfTable)
{
caseTable.Add(en.Key, en.Value);
if (item.Key is not string s)
continue;

caseTable.Add(s, item.Value);
}
_caseInsensitiveTable = caseTable;
}

return caseTable[name];
caseTable.TryGetValue(name, out object? value);
return value;
}
}
}
64 changes: 64 additions & 0 deletions src/libraries/System.Resources.ResourceManager/tests/ResourceSetTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.IO;
using System.Reflection;
using System.Globalization;
using System.Collections;
using System.Collections.Generic;

namespace System.Resources.Tests
Expand Down Expand Up @@ -101,6 +102,69 @@ public void GetString()
}
}

public class ResourceSetTests_IResourceReader
{
class SimpleResourceReader : IResourceReader
{
Hashtable data;
public SimpleResourceReader()
{
data = new Hashtable();
data.Add(1, "invalid");
data.Add("String", "message");
data.Add("Int32", 5);
}

IEnumerator IEnumerable.GetEnumerator()
{
throw new NotSupportedException();
}

public IDictionaryEnumerator GetEnumerator()
{
return data.GetEnumerator();
}

public void Close()
{
}

public void Dispose()
{
}
}

[Fact]
public void Empty_Ctor()
{
Assert.Throws<ArgumentNullException>(() => new ResourceSet(null as IResourceReader));
}

[Fact]
public void GetObject()
{
var rs = new ResourceSet(new SimpleResourceReader());

Assert.Null(rs.GetObject("DoesNotExist"));
Assert.Null(rs.GetObject("1"));

Assert.Equal(5, rs.GetObject("Int32"));
Assert.Equal(5, rs.GetObject("int32", true));
}

[Fact]
public void GetString()
{
var rs = new ResourceSet(new SimpleResourceReader());

Assert.Null(rs.GetString("DoesNotExist"));
Assert.Null(rs.GetString("1"));

Assert.Equal("message", rs.GetString("String"));
Assert.Equal("message", rs.GetString("string", true));
}
}

public class ResourceSetTests_StreamCtor : ResourceSetTests
{
public override ResourceSet GetSet(string base64Data)
Expand Down

0 comments on commit a683543

Please sign in to comment.