Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Dictionary for underlying cache of ResourceSet #44104

Merged
merged 2 commits into from
Nov 6, 2020
Merged

Use Dictionary for underlying cache of ResourceSet #44104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
052f8eb
Use Dictionary for underlying cache of ResourceSet
marek-safar Oct 31, 2020
a50326f
Use object type for key types
marek-safar Nov 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 43 additions & 64 deletions src/libraries/System.Private.CoreLib/src/System/Resources/ResourceSet.cs
View file
Open in desktop
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
Open in desktop
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