XmlReader Question

[Mitch]

Hello,

I have a singleton class that loads a config file (for an assembly) into an
internal stream object. I designed this class as singleton because it
parses the config file many times and I do not want to load the config file
everytime. The problem is I am using XMLReader because of speed and after I
read an item (multiple nodes so pass node name and element name) and try to
read another item I get an error "root element missing". Is the error
caused by XMLReader object or should I change this a load the config file
into XMLDocument and parse it. Am I even acheiving that much performance
gain using XMLReader? Any ideas?

public string GetXMLValue(string nodeName, string elementName)
{
string NS = "";
string xmlValue = "";
XmlReaderSettings settings = new XmlReaderSettings();
settings.IgnoreWhitespace = true;
settings.ProhibitDtd = true;
settings.IgnoreComments = true;
settings.IgnoreProcessingInstructions = true;
settings.ConformanceLevel = ConformanceLevel.Document;
XmlReader reader = null;
using (reader = XmlReader.Create(mappingStream, settings))
{
try
{
reader.Read();
reader.ReadToDescendant(nodeName, NS);
while (reader.ReadToFollowing(elementName, NS))
{
xmlValue = reader.ReadElementContentAsString();
}
return xmlValue;
}
catch
{
return xmlValue;
.....
}
}
}

 

[_xarky]

If you are using the dot net app.config files, then take a look at the
ConfigurationManager class. This would avoid you from personally
parsing the xml file directly.

 

[Marc Gravell]

It doesn't look like you ever rewind the stream; so either set
mappingStream.Position = 0, or recreate the stream each call (I gather
you don't want to do this latter).

Note also that this code is not thread-safe (which static methods
almost always [or: always] should be); if two threads try to read from
the same stream at the same time you are going to be in a world of
hurt.

As for performance; it may depend on the size of the source xml;
parsing large xml into a DOM to read one value can be expensive...
however, if you only do it once and then query the DOM, then a DOM may
be quicker - but it will take more memory. There is also a readonly
DOM/XPath variant (meant to be less bloated), but see the other post
about the MS-provided wrappers first...

As a final thought: how often do you query the configuration settings?
The reason I ask is that this may be a premature optimisation; you
could spend lots of time tweaking this code, only to find it makes
almost no difference to your system's overall performance; I'd check
for the actual bottlenecks (using a profiler) before I got too excited
about making this the model of perfection (finite developer time, etc;
do the things that matter).

Marc

 

[Marc Gravell]

Another thought; you could always cache values in a
Dictionary<string,string>

public string GetXMLValue(string nodeName, string elementName)
{
lock(valueDictionary) {
string value, key = (nodeName ?? "") + "#" + (elementName ?? "");
// that'll do...
if(!valueDictionary.TryGetValue(key, out value)) {
value = YourExistingLookupMethod(nodeName,elementName);
valueDictionary.Add(key, value);
}
return value;
}
}

Note that the lock in here makes it thread-safe, so there would be no
need to make YourExistingLookupMethod thread-safe unless it was public;
however, now a: the stream is only hit for unread values, and b: we
only store the ones in use.

Marc

 

[Mitch]

Marc,

Thanks for the help. I am using this singleton class in an assembly used in
a web application. The config file holds data that used constantly by the
assembly. Initially, I design the class not as static and had the config
file loaded each time the class was created however I thought maybe a
singleton class would help performance and have it load the config file
once. However, as you pointed out that it is not thread safe. I could go
back to my initial design however I rather not. I could move this data into
the web.config file. I was curious what tools like O/R Mappers that
mappings do. I am thinking that they load the mapping into a
hashtable/dictionary like you suggested. If I load it into a XMLDocument
initially would this solve my problem? Any other suggestions?

 

[Marc Gravell]

...maybe a singleton class...
"static" and "singleton" are very similar concepts - at least in terms
of the overall effect, such as thread safety - and note that a
web-server app is one of those *most likely* to suffer from thread
safety issues.

...O/R Mappers...

OR mappers have to do with db <==> entity conversions; I'm not sure
that the OR mapper concept would specify much re implementation details
like hashtables, although I'm sure some do... but this doesn't feel
quite like what you are after.
http://en.wikipedia.org/wiki/Object-relational_mapping

...load it into a XMLDocument...

A static/singleton instance of an XmlDocument (or XPathDocument)
[initialised via (for instance) the static/singleton ctor] would
probably work, as long as no thread updates the data... if it does,
you'd still need to sync the threads.

...not thread safe...

But it can be *made* thread safe, as my example demonstrated

...holds data that used constantly by the assembly...
...Any other suggestions?...

If it is in constant use, then perhaps you are being overly complex in
your solution; perhaps simpler is better here... you could (in the
static/singleton ctor) read the config file, parsing the salient
settings into readonly fields/properties on your class. This would
provide type safety, thread safety, and maximum performance [since
readonly, likely to be "public SomeType SomeProperty {get {return
someField;}}" which "inlines" during JIT very nicely indeed]. Cheaper
than looking it up every time - although less flexible, as new settings
require new properties.
Alternatively, look at the managed configuration classes.

Marc

 

[Mitch]

Marc,

Thanks for all your help. I think you misunderstood me about OR mappers. I
was pointing out that the mapper uses a mappings file that is continuely
accessed. My thought is that I think the mappings file is loaded into some
sort of object that you have suggested. I decided not move to the idea of
moving creating a class that represents the file. I decided to move a way
from the static/singleton design and refactored my assembly to create one
object per instance of my assembly. This makes it thread safe plus it
eliminates the constant loading of the config file.

Thanks again

...maybe a singleton class...

"static" and "singleton" are very similar concepts - at least in terms
of the overall effect, such as thread safety - and note that a
web-server app is one of those *most likely* to suffer from thread
safety issues.

...O/R Mappers...

OR mappers have to do with db <==> entity conversions; I'm not sure
that the OR mapper concept would specify much re implementation details
like hashtables, although I'm sure some do... but this doesn't feel
quite like what you are after.
http://en.wikipedia.org/wiki/Object-relational_mapping

...load it into a XMLDocument...

A static/singleton instance of an XmlDocument (or XPathDocument)
[initialised via (for instance) the static/singleton ctor] would
probably work, as long as no thread updates the data... if it does,
you'd still need to sync the threads.

...not thread safe...

But it can be *made* thread safe, as my example demonstrated

...holds data that used constantly by the assembly...
...Any other suggestions?...

If it is in constant use, then perhaps you are being overly complex in
your solution; perhaps simpler is better here... you could (in the
static/singleton ctor) read the config file, parsing the salient
settings into readonly fields/properties on your class. This would
provide type safety, thread safety, and maximum performance [since
readonly, likely to be "public SomeType SomeProperty {get {return
someField;}}" which "inlines" during JIT very nicely indeed]. Cheaper
than looking it up every time - although less flexible, as new settings
require new properties.
Alternatively, look at the managed configuration classes.

Marc