Performance of GetCustomAttributes (Reflection)

[jclarknet]

I was taking a look at the sharp architecture and the domain object
class to try a few benchmarks against a DDD app I'm building, and
found a serious performance issue that really wasn't that
surprising. In Sharp architecture you use attributes to flag the
properties which make up the domain signature for the object, although
this is great for code re-use and not cluttering up domain objects
with specific implementations of GetHashCode, the use of
GetCustomAttributes is terrible for anything that's under serious
load. I ran a test on a simple asp.net app and over 60% of the time
was spent in GetHashCode more specifically GetCustomAttributes. I
had two-fold increase in performance when commenting out the code that
gets the custom attributes.

I'm assuming that folks using this are aware of the performance
penalty of using reflection, from what I can see the only way to
improve performance is to implement specific versions of GetHashCode
in each domain object.

If anyone has any feedback, I'd love to hear it.

Cheers.

[Simone Busoli]

I think that a recent commit switched from Type.GetCustomAttributes to Attribute.IsDefined, can you check it out?

[Luis Abreu]

Yeah, one of the last releases has that.

 

Btw, and since I’m no reflection guru, I’m curious: what’s the most expensive part? Get the attributes or getting the value from the prop info?

 

---

Luis Abreu

[Simone Busoli]

It's not a release, it's one of the latest commits in the repository.

Anyways, now that I look at the code, I think we have an issue with the implementation of the SignatureProperties property. I'm giving it a shot right now.

[Simone Busoli]

That's fixed now. Signature properties were evaluated too much lazily, that is, each time its getter was invoked the lambda which looked up the DomainSignature attributes was executed due to the lazy nature of LINQ.

Now they are cached per each instance of each type. A future improvement might be to have a cache by type.

[Luis Abreu]

Ok, ok…one of the latest commits.

 

That didn’t answer my question: is getting the value through relfection less expensive than getting the property info? If it is, then caching that info would be an option, right? (but only after that is confirmed)…

[Simone Busoli]

The expensive part is the binding, once the property is bound retrieving its value is acceptable. Actually, type's members are cached by the fx, so subsequent queries would be fast as well, but we're already caching the properties which make up the signature, so we don't need to care about that.

[jclarknet]

I'll grab the latest and try it out, the expensive part is the
GetCustomAttributes call, at least thats what JetBrains profiler said.

> > Cheers.- Hide quoted text -
>
> - Show quoted text -

[Simone Busoli]

Actually, there were two issues previously, one was certainly that GetCustomAttributes is slower that Attribute.IsDefined, but mainly because we didn't cache the computation.

[Billy]

For those that haven't looked at the change Simone's change, for
getting the SignatureProperties, was as follows:

return domainSignatureProperties ?? (domainSignatureProperties =
GetType().GetProperties()
.Where(p => Attribute.IsDefined(p, typeof
(DomainSignatureAttribute), true)).ToList());

Very clever and should hopefully resolve the performance issue.
Thanks Simone!

@jclarknet,

Would you be able to run your tests again to see if this has provided
a suitable performance gain?

Thank you!
Billy

On Jan 4, 3:04 am, "Simone Busoli" <simone.bus...@gmail.com> wrote:
> Actually, there were two issues previously, one was certainly that
> GetCustomAttributes is slower that Attribute.IsDefined, but mainly because
> we didn't cache the computation.
>

[Marco]

Why not make the domainSignatureProperties static so it will only be
filled once for each type?

[Simone Busoli]

Because we're leveraging the polymorphic behavior of GetType().

[Simone Busoli]

Actually, caching them in a dictionary static field could be a performance improvement. As they are now, they are cached for each instance, but different instances of the same type won't share the knowledge.

[Simone Busoli]

Committed. Now we have a single lookup per type, then it's cached.

[Billy]

That's a slick change Simone!

Billy

On Jan 7, 4:08 pm, "Simone Busoli" <simone.bus...@gmail.com> wrote:
> Committed. Now we have a single lookup per type, then it's cached.
>

> On Wed, Jan 7, 2009 at 11:24 PM, Simone Busoli <simone.bus...@gmail.com>wrote:
>
> > Actually, caching them in a dictionary static field could be a performance
> > improvement. As they are now, they are cached for each instance, but
> > different instances of the same type won't share the knowledge.
>

> > On Wed, Jan 7, 2009 at 11:11 PM, Simone Busoli <simone.bus...@gmail.com>wrote:
>
> >> Because we're leveraging the polymorphic behavior of GetType().
>

[Luis Abreu]

Simone, about this caching thing…

 

I’ve tried doing something like that with value objects where I’d cache the list of properties by type on a dictionary. I had a friend that run some tests and he told me that there really wasn’t much improvement with the caching of the properties. That’s why I asked you in a previous mail what was the most expensive part (getting the Propertyinfo or getting the value of the propertyinfo…)

 

 

---

Luis Abreu

 

From: sharp-arc...@googlegroups.com [mailto:sharp-arc...@googlegroups.com] On Behalf Of Simone Busoli
Sent: quarta-feira, 7 de Janeiro de 2009 22:25
To: sharp-arc...@googlegroups.com
Subject: Re: Performance of GetCustomAttributes (Reflection)

 

Actually, caching them in a dictionary static field could be a performance improvement. As they are now, they are cached for each instance, but different instances of the same type won't share the knowledge.

[Simone Busoli]

I didn't do any performance tests on that, but I think that expensive or not, there's no reason to look up the properties of the signature for each instance of the same type, and since "this caching thing" is trivial I choose to put it in. I think a well done performance test would show a sensible improvement btw.

[Luis Abreu]

Ok.

 

I too don’t have the time to write such test now so I’ll see if my friend still has that demo code that he has written…

[Simone Busoli]

So here's the test code (I ran it in LinqPad):

Func<Type, IEnumerable<PropertyInfo>> getProps = t => t.GetProperties();

Func<Type, IEnumerable<PropertyInfo>> getPropsMemoized = ((Func<Func<Type, IEnumerable<PropertyInfo>>>)(() => {

var values = new Dictionary<Type, IEnumerable<PropertyInfo>>();

return t => {

if(values.ContainsKey(t))

return values[t];

var value = getProps(t);

values[t] = value;

return value;

};

}))();

Action<Action<Type>, string> measureTime = (action, name) => {

"".Dump(name);

foreach(var t in typeof(int).Assembly.GetExportedTypes().Where(t => t.GetProperties().Count() > 4).Take(10))

{

var watch = Stopwatch.StartNew();

foreach(var i in Enumerable.Range(0, 100000)) action(t);

(t.Name + ": " + watch.Elapsed.ToString()).Dump();

}

};

measureTime(t => getPropsMemoized(t), "memoized");

measureTime(t => getProps(t), "non memoized");

[Frank]

Does this implementation have a negative impact on multiple threads?
I'm assuming that the dictionary is thread-safe, but will there be
lock contention issues with this approach? I'm wondering if
introducing a lock for every request is worse than just recalculating
on every request. Thoughts?

-Frank

On Jan 7, 2:24 pm, "Simone Busoli" <simone.bus...@gmail.com> wrote:
> Actually, caching them in a dictionary static field could be a performance
> improvement. As they are now, they are cached for each instance, but
> different instances of the same type won't share the knowledge.
>

> On Wed, Jan 7, 2009 at 11:11 PM, Simone Busoli <simone.bus...@gmail.com>wrote:
>
> > Because we're leveraging the polymorphic behavior of GetType().
>

[Billy]

Frank,

This is a great concern to bring up. Do you know of a way for us to
be able to test or verify if this might pose a problem? (I have to
admit that I'm not very experienced with dealing with multi-
threading.)

Thank you,
Billy

[Michael Tsibelman]

Actualy no locks are need it in this scenario. If you call SignatureProperies from multiple threads in exactly the same time what will happen is that each thread will execute this line:

GetType().GetProperties().Where(p => Attribute.IsDefined(p, typeof(DomainSignatureAttribute), true)).ToList();

and will put result in the dictionary under the same key overriding the previous value, because we know for sure what this result would be identical in each execution it not a problem. It's just a slight performance hit for few very first concurrent calls to this property.  If you still think that it better to optimize for this few calls I can recommend following implementation:

 

private static readonly Object padlock = new Object();

 

public override IEnumerable<PropertyInfo> SignatureProperties {

    get {

        IEnumerable<PropertyInfo> properties;

       

        if (signaturePropertiesDictionary.TryGetValue(GetType(), out properties))

            return properties;

       

    properties=GetType().GetProperties()

.Where(p => Attribute.IsDefined(p, typeof(DomainSignatureAttribute), true))

.ToList();

 

 

        lock (padlock)

        {

            signaturePropertiesDictionary[GetType()] = properties;

        }

 

        return properties;

    }

}

 

[Luis Abreu]

Hello again.

 

Am I wrong when I look at the code and say that you’re just getting the list of properties?

[Luis Abreu]

Anything wrong with the double lock strategy?

 

---

Luis Abreu

 

From: sharp-arc...@googlegroups.com [mailto:sharp-arc...@googlegroups.com] On Behalf Of Michael Tsibelman
Sent: sexta-feira, 9 de Janeiro de 2009 06:53
To: sharp-arc...@googlegroups.com
Subject: Re: Performance of GetCustomAttributes (Reflection)

 

Actualy no locks are need it in this scenario. If you call SignatureProperies from multiple threads in exactly the same time what will happen is that each thread will execute this line:

[Simone Busoli]

Yes, that's what we're talking about. Getting their values cannot be cached since it might change.

[Simone Busoli]

From the docs:

A Dictionary<TKey, TValue> can support multiple readers concurrently, as long as the collection is not modified. Even so, enumerating through a collection is intrinsically not a thread-safe procedure. In the rare case where an enumeration contends with write accesses, the collection must be locked during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

The potential issues with the code in the repository are several:

- writes are not thread safe, and the test program below shows that. Running it on a dual core machine prints less than 100000. Not a big deal, but locking the write operation as you did solves this.

- however, locking on writes still doesn't solve the issue in writing twice on the same key. Not a big deal again, but to accomplish this, wee need to perform a double check lock as suggested by Luis.

- even if we serialize writes, the docs say that concurrent reads and writes are not safe, but I extended the test program by making reads and writes concurrently and no issues emerged, so I'm not sure if we need to synchronize reads as well. In this case, instead of locking we could use a ReaderWriterLock(Slim), but I'm not sure we really need this. Ideally, the dictionary should have few writes and many more reads.

Here's the test program, which shows that concurrent writes are not safe:

    class Program

    {

        private static void Main()

        {

            var handle = new ManualResetEvent(false);

            var dictionary = new Dictionary<int, int>();

            var iterations = 0;

            const int maximumCount = 100000;

            var syncLock = new object();

            for (var i = 0; i < maximumCount; i++)

            {

                var temp = i;

                ThreadPool.QueueUserWorkItem(state =>

                {

                    handle.WaitOne();

//                    This is a bug: uncommenting this will always print 100000, which is right

//                    lock (syncLock)

                        dictionary[temp] = temp;

                    Interlocked.Increment(ref iterations);

                });

            }

            handle.Set();

            while (iterations < maximumCount)

                Thread.Sleep(100);

            Console.WriteLine("Count: " + dictionary.Count);

        }

    }

[Michael Tsibelman]

Alternative is just to move the dictionary intialization into static constuctor. This way it will be called only once on first reference of a class, we loose LazyLoading benefit but i think it's very small compared for the overhead of syncronization.

[Simone Busoli]

What do you mean? I'm hoping not something like

static DomainObject()

{

  foreach(Type t derived from DomainObject)

   dictionary[t] = signature properties of t;
}

[Michael Tsibelman]

I forgot that GetType() is not  a static method :)

[Frank]

Here's what I find from MSDN for the docs on Dictionary<K,V>:

A Dictionary<(Of <(TKey, TValue>)>) can support multiple readers

concurrently, as long as the collection is not modified. Even so,
enumerating through a collection is intrinsically not a thread-safe
procedure. In the rare case where an enumeration contends with write
accesses, the collection must be locked during the entire enumeration.
To allow the collection to be accessed by multiple threads for reading
and writing, you must implement your own synchronization.

Anytime you have static members in a web application, you have to
consider whether or not these members might be accessed from multiple
requests. Because the Dictionary class by default is not thread-safe
(meaning it does not guarantee that any methods are done in an atomic
way), then it means that users of the class have to ensure that it is
thread-safe. The only real concern in this situation is the generation
of the cached property metadata (not the values of the properties of
course), which really should only happen once at the beginning of the
world. So, one strategy is to try and pre-cache all of these before
any requests come in. Not sure how to accomplish this other than
perhaps hooking the Application_Init event from the HttpApplication.
Alternatively, you'll have to lock any access to the static
Dictionary, both on the reading and the writing, which therefore means
that every single request would have to hit the lock (not just the
startup case). This would obviously have a very negative impact on
scalability.

So what's the danger of not making the Dictionary thread-safe? Well,
imagine you just deployed the application to the web server and in
some stroke of fate, two users decide to view the site at the exact
same time. There's a potential for both users to get different
signatures for a particular type. At some point, one thread might
think that the dictionary already contains the metadata, while the
other thread is in the process of caching the metadata.

How do you test for it? Gosh, that's a tough one, it's never easy.
Stress tests, but that could even fail, depending on how many CPUs the
machine has, depending on the alignment of the planets, etc. The trick
is trying to get the two threads to execute certain portions of the
internal methods of the Dictionary at the exact same time.

BTW, if you're curious about this sort of issue, do a google search
for "thread safe singleton". The thread-safe singleton problem is very
similar to the problem of having static members accessed by multiple
requests or almost any caching strategy.

-Frank

[Billy]

There's actually an example of a thread safe singleton already in
S#arp Architecture (the only singleton in #arch incidentally) at
http://sharp-architecture.googlecode.com/svn/trunk/src/SharpArch/SharpArch.Data/NHibernate/DbContext.cs.

Frank, would it be your suggestion to do away with the property
caching or to create a thread safe singleton to hold the cached types?

Thanks again!
Billy

[Frank]

Actually, I just came to the idea that Thread-Local Storage might be
the best way to go. Of course, you'll need to abstract the TLS aspect
from the domain, so I'd probably use a repository pattern for anything
that needs to be cached from inside the domain. In this case, TLS
works great because it means you get:

- %100 reliability
- No performance degradation from the need for locks
- TLS is lightweight

What do you think?
-Frank

On Jan 9, 8:58 am, Billy <googlegro...@emccafferty.com> wrote:
> There's actually an example of a thread safe singleton already in

> S#arp Architecture (the only singleton in #arch incidentally) athttp://sharp-architecture.googlecode.com/svn/trunk/src/SharpArch/Shar....

[Billy]

That makes perfect sense. So something along these lines
http://www.dotnetjunkies.com/WebLog/chris.taylor/archive/2005/08/18/132026.aspx
?

Billy

[Frank]

Yeah! I didn't know about [ThreadStatic], but that seems to be the
trick. There are many places I could use something like that without
having to write all the TLS junk myself.

On Jan 9, 1:10 pm, Billy <googlegro...@emccafferty.com> wrote:
> That makes perfect sense.  So something along these lineshttp://www.dotnetjunkies.com/WebLog/chris.taylor/archive/2005/08/18/1...

[Billy]

Voila...tis now ThreadStatic:
http://sharp-architecture.googlecode.com/svn/trunk/src/SharpArch/SharpArch.Core/DomainObject.cs

http://sharp-architecture.googlecode.com/svn/trunk/bin/Sharp.Core.dll
now includes this modification.

Billy

[Billy]

Er...that's the SharpArch.Core.dll in the bin directory.

Billy

On Jan 9, 3:41 pm, Billy <googlegro...@emccafferty.com> wrote:
> Voila...tis now ThreadStatic:http://sharp-architecture.googlecode.com/svn/trunk/src/SharpArch/Shar...

> ...
>
> read more »

[Billy]

It appears that the introduction of the ThreadStatic has introduced a
very pedagogical bug (sorry, I love that word). I've been developing
against this for an hour or so and have run into an "Object reference
not set to an instance of an object" error a couple of times on the
following line:

if (signaturePropertiesDictionary.TryGetValue(GetType(), out
properties))
return properties;

I'm suspecting that the following is occurring:

1) The private signaturePropertiesDictionary is getting initialized to
a new dictionary on the first thread that the application is running
on, as follows:

[ThreadStatic]
private static readonly Dictionary<Type,
IEnumerable<PropertyInfo>> signaturePropertiesDictionary =

new Dictionary<Type, IEnumerable<PropertyInfo>>();

2) The application switches to using a new thread. Since
signaturePropertiesDictionary exists on each thread, the "new
Dictionary" isn't being invoked on the thread switch.

3) signaturePropertiesDictionary is now null on the new thread because
it's only not-null on the thread that initialized the object.

The unit tests (nor a little simple web testing) didn't show the
problem because the odds were low (impossible with the unit tests
unless explicitly done) that the thread would be switched during
execution.

I've changed the code within DomainObject.cs to the following which
appears to have fixed the problem:

public override IEnumerable<PropertyInfo> SignatureProperties {
get {
IEnumerable<PropertyInfo> properties;

if (signaturePropertiesDictionary == null)
signaturePropertiesDictionary = new Dictionary<Type,
IEnumerable<PropertyInfo>>();

if (signaturePropertiesDictionary.TryGetValue(GetType(), out
properties))
return properties;

return (signaturePropertiesDictionary[GetType()] =
GetType().GetProperties().Where(p => Attribute.IsDefined
(p, typeof(DomainSignatureAttribute), true)).ToList());
}
}

[ThreadStatic]
private static Dictionary<Type, IEnumerable<PropertyInfo>>
signaturePropertiesDictionary;

The source on trunk has been updated, accordingly, along with the
SharpArch.Core.dll in /trunk/bin.

Billy McCafferty

> ...
>
> read more »

[Billy]

And sure enough, that was the problem...
http://blogs.msdn.com/jfoscoding/archive/2006/07/18/670497.aspx

How cool of a bug was that?!?!? ;)

Billy

> ...
>
> read more »

[Simone Busoli]

Can you explain why making it thread static is good? So each thread has its own dictionary, why is it better than having the signature properties recalculated per instance as it was before?

[Simone Busoli]

In addition to the fact that I don't really see the point in storing the dictionary in every thread, to the extent that then I would simply leave it per instance, you should really read this.

[Billy]

It seems like some benchmarking is in order to see if this is pulling
its weight. Have we "over optimized," is it worth it?

Billy

On Jan 10, 6:27 am, "Simone Busoli" <simone.bus...@gmail.com> wrote:
> In addition to the fact that I don't really see the point in storing the
> dictionary in every thread, to the extent that then I would simply leave it
> per instance, you should really read

> this<http://piers7.blogspot.com/2005/11/threadstatic-callcontext-and_02.html>
> .

> On Sat, Jan 10, 2009 at 12:50 PM, Simone Busoli <simone.bus...@gmail.com>wrote:
>
> > Can you explain why making it thread static is good? So each thread has its
> > own dictionary, why is it better than having the signature properties
> > recalculated per instance as it was before?
>

> ...
>
> read more »

[Simone Busoli]

I'm still wondering what's the rationale behind giving each thread its copy of the dictionary.

[Billy]

The static variable isn't threadsafe without threadstatic.

Billy

On Jan 10, 6:36 am, "Simone Busoli" <simone.bus...@gmail.com> wrote:
> I'm still wondering what's the rationale behind giving each thread its copy
> of the dictionary.
>

> ...
>
> read more »

[Simone Busoli]

Sure. ThreadStatic (1) is thread safe, as is storing the IEnumerable<PropertyInfo> (2) in an instance field. So why is 1 better than 2?

[Frank]

Because in most web application hosting environments (ie ASP.NET)
threads are pooled. This pool is allocated on demand, meaning that the
number of threads in the pool only goes up when it is needed, and once
the threads are created, they generally don't go away. The pool also
has an upper bound so that the number of threads never goes beyond a
certain limit. What this all means is that with [ThreadStatic] the
caching behavior will happen much more rarely than on every request.

The article you reference is really talking about the improper usage
of [ThreadStatic]; using it for request-level data. In that case, the
data might change per request, so storing it in a [ThreadStatic], or
even a static for that matter doesn't make sense. In our case, this is
a WORM (write-once-read-many).

To me, this is a good balance between performance and reliability. In
my view, reliability is paramount for any development of a library, so
performance comes second. This also enforces the design goal of
keeping the hosting environment out of the domain; this accomplishes
that. A [ThreadStatic] should work in any environment, assuming that
processing happens on reusable threads.

On Jan 10, 5:40 am, "Simone Busoli" <simone.bus...@gmail.com> wrote:
> Sure. ThreadStatic (1) is thread safe, as is storing the
> IEnumerable<PropertyInfo> (2) in an instance field. So why is 1 better than
> 2?
>

> ...
>
> read more »

[Simone Busoli]

Fine. That's a good explanation and what I wanted to hear. So my point is that, as an application developer, I wouldn't want any of the libraries I'm using to store anything in MY threads.

[Simone Busoli]

Since no one is coming up with a clever idea, I'll throw mine:

public class DomainObject<T> where T : DomainObject<T>

{

static IEnumerable<PropertyInfo> properties = 

new List<PropertyInfo>(typeof(T).GetProperties(BindingFlags.Public | BindingFlags.Instance)

.Where(p => Attribute.IsDefined(p, typeof(DomainSignatureAttribute))));

}

1. no threading issues

2. no redundancy in memory occupation

This works because of the c# specification:

A static variable in a generic class declaration is shared amongst all instances of the same closed constructed type, but is not shared amongst instances of different closed constructed types. These rules apply regardless of whether the type of the static variable involves any type parameters or not.

[Frank]

I really like that you're using the CRTP (Curiously-Recurring Template
Pattern). It removes the need to have a static Dictionary, which is
definitely better, but I'm failing to see how this would remove any of
the threading issues. My understanding was that static initializers
are not guaranteed to be executed in a thread-safe fashion.

OK, I take that back; this is why I like this forum, it always teaches
me new things. In the C++ world, static initializers are not thread-
safe, but they apparently are in C#, with some gotchas:

http://www.yoda.arachsys.com/csharp/beforefieldinit.html
http://www.ondotnet.com/pub/a/dotnet/2003/07/07/staticxtor.html

So, you might want to use a static constructor instead of using a
static field initializer due to some dirty details of the JIT.

I think this is a great solution! Let's see how people respond to the
strange looking syntax when deriving from DomainObject<T> (I'm used to
it).

On Jan 10, 5:26 pm, "Simone Busoli" <simone.bus...@gmail.com> wrote:
> Since no one is coming up with a clever idea, I'll throw mine:
> public class DomainObject<T> where T : DomainObject<T>
> {
> static IEnumerable<PropertyInfo> properties =
> new List<PropertyInfo>(typeof(T).GetProperties(BindingFlags.Public |
> BindingFlags.Instance)
> .Where(p => Attribute.IsDefined(p, typeof(DomainSignatureAttribute))));
>
> }
>
> 1. no threading issues
> 2. no redundancy in memory occupation
>
> This works because of the c# specification:
>
> A static variable in a generic class declaration is shared amongst all
> instances of the same closed constructed type, but is not shared amongst
> instances of different closed constructed types. These rules apply
> regardless of whether the type of the static variable involves any type
> parameters or not.
>

> On Sat, Jan 10, 2009 at 9:44 PM, Simone Busoli <simone.bus...@gmail.com>wrote:
>
> > Fine. That's a good explanation and what I wanted to hear. So my point is
> > that, as an application developer, I wouldn't want any of the libraries I'm
> > using to store anything in MY threads.
>

[Simone Busoli]

I went into Ritcher's book right now and nowhere is mentioned that static field initializers are not guaranteed to be thread-safe. And I'm pretty sure this is the case, or the classic singleton pattern which initializes the instance of the singleton in a field initializer wouldn't might not work correctly.

The difference between type initializers and static field initializers is the timing in which they are executed. In this case, I don't think this makes any difference, and personally I prefer instantiating the collection of signature properties in the field initializer for better readability.

About the "dirty details of the JIT", you might have issues if you use type initializers (static constructor), not static field initializers. CLR via C# pg. 193-195.

[Luis Abreu]

I know that. What I was trying to say was that the most expensive part of getting the value through reflection was getting the value from a propertyINfo object and not the act of getting the propertyinfo instance in the first place….

[Luis Abreu]

Is is that expensive to use a lock instead of using the threadstatic attribute? Just asking…

 

From: sharp-arc...@googlegroups.com [mailto:sharp-arc...@googlegroups.com] On Behalf Of Simone Busoli
Sent: sábado, 10 de Janeiro de 2009 13:41
To: sharp-arc...@googlegroups.com
Subject: Re: Performance of GetCustomAttributes (Reflection)

 

Sure. ThreadStatic (1) is thread safe, as is storing the IEnumerable<PropertyInfo> (2) in an instance field. So why is 1 better than 2?

[Luis Abreu]

Seems good to me.

 

From: sharp-arc...@googlegroups.com [mailto:sharp-arc...@googlegroups.com] On Behalf Of Simone Busoli
Sent: domingo, 11 de Janeiro de 2009 01:26
To: sharp-arc...@googlegroups.com
Subject: Re: Performance of GetCustomAttributes (Reflection)

 

Since no one is coming up with a clever idea, I'll throw mine:

[Simone Busoli]

Luis, what could we do about that? The values have to be recalculated anyways. The point is trying to avoid to look up the signature properties. There's no caching on the values that we could perform.

Maybe an optimization would be composing a list of Expressions instead of getting the values by reflection, but that's not the point of this discussion.

[Billy]

I like this solution as well. One worry I had about the ThreadStatic
solution was that, by default, ASP.NET 2.0 applications (and up?),
allocate up to 100 threads to be available for handling the
application. While I doubt that more than a few dozen would be used,
even under good sized loads, I felt a little uncomfortable about
having an indeterminate sized collection of types placed onto each and
every thread that processes the application. Anyway, I wonder how the
newly suggested solution would work with inheitance structures.

Suppose you had a Person object which derives from
DomainObject<Person>. Now suppose further that you had Customer and
Employee objects, both inheriting from Person. It seems that the
solution presented would dictate that the domain signature cannot be
modified in the Customer and Employee objects. Am I missing an easy
way around this limitation?

Billy

On Jan 11, 4:38 am, Frank <frank.l...@gmail.com> wrote:
> I really like that you're using the CRTP (Curiously-Recurring Template
> Pattern). It removes the need to have a static Dictionary, which is
> definitely better, but I'm failing to see how this would remove any of
> the threading issues. My understanding was that static initializers
> are not guaranteed to be executed in a thread-safe fashion.
>
> OK, I take that back; this is why I like this forum, it always teaches
> me new things. In the C++ world, static initializers are not thread-
> safe, but they apparently are in C#, with some gotchas:
>

> http://www.yoda.arachsys.com/csharp/beforefieldinit.htmlhttp://www.ondotnet.com/pub/a/dotnet/2003/07/07/staticxtor.html

[Simone Busoli]

I'm afraid you're not.

[Luis Abreu]

Well, what I’m saying is that we’re going to so much trouble in optimizing the code in getting the props when the most expensive operation might be getting the values. That’s all I’m saying…

[Frank]

Ouch, that's a good point! Multiple levels of inheritance won't work.

But let's look at it like this, if the server had 100 threads in use,
then I'd say you're doing something right; your site is wildly
popular. Another perspective is that if we go back to doing a per-
instance cache, you potentially are using more memory than the
[ThreadStatic] approach. What is the real impact of not caching at
all? It feels like this whole things opens a real can of worms.

> >http://www.yoda.arachsys.com/csharp/beforefieldinit.htmlhttp://www.on...

[Billy]

WRT this issue, let's close it by sticking with the current
implementation that's on the trunk:
http://sharp-architecture.googlecode.com/svn/trunk/src/SharpArch/SharpArch.Core/DomainObject.cs

Billy

[Simone Busoli]

I'm fine with this, but I think a synchronization construct would be more appropriate. Using thread local storage is just a workaround.

[Billy]

Hi Simone,

Please let me know if you have a specific implementation suggestion.

Thanks!
Billy

On Jan 11, 4:36 pm, "Simone Busoli" <simone.bus...@gmail.com> wrote:
> I'm fine with this, but I think a synchronization construct would be more
> appropriate. Using thread local storage is just a workaround.
>

> On Sun, Jan 11, 2009 at 8:19 PM, Billy <googlegro...@emccafferty.com> wrote:
>
> > WRT this issue, let's close it by sticking with the current
> > implementation that's on the trunk:
>

> >http://sharp-architecture.googlecode.com/svn/trunk/src/SharpArch/Shar...

[Simone Busoli]

Yes, a reader writer lock, although it looks like synchronizing access to a dictionary is tricky for microsoft as well: http://weblogs.asp.net/leftslipper/archive/2008/03/31/mvc-locking-the-routecollection.aspx