Re: GetCurrentMethod possibly returning the wrong method (was Re: Raising property changed events)

2011-03-24 Thread David Burela
Had no idea about that. Thanks for following it up!

On 25 March 2011 11:19, Mark Hurd markeh...@gmail.com wrote:

 Thanks for that clarification. However GoogleDesktop found my previous
 recollection discussing this was in the Microsoft NewsGroups, which
 now Googling for the subject of that exchange Reflection and compiler
 inlining found this other reply:

 On 2009-12-11 6:18, Alex Clark wrote:
  Yes, because any method that calls MethodBase.GetCurrentMethod() will not
 be
  inlined. This is because .GetCurrentMethod() includes a StackCrawlMark, a
  special magical enum for methods that need to walk the stack (like
  .GetCurrentMethod() predictably needs, to look for its caller) that
 prevents
  the caller from being inlined. Trying to be clever by sticking the call
 in a
  delegate like Mark did will not upset this, because methods that call
 delegates
  are not inlined either.

 (as retrieved from

 http://www.eggheadcafe.com/software/aspnet/35469613/reflection-and-compiler-inlining.aspx
 but there are a number of clones of this info)

 So have things changed or was Alex Clark wrong?
 --
 Regards,
 Mark Hurd, B.Sc.(Ma.)(Hons.)

 On 25 March 2011 02:40, David Kean david.k...@microsoft.com wrote:
  I chased this up with one the devs on the JIT team. He confirmed that the
 JIT/NGEN doesn't give this guarantee, both inlining and tail calls can cause
 Assembly.GetExecutingAssembly, Assembly.GetCallingAssembly and
 Method.GetCurrentMethod to return incorrect results. You can somewhat
 mitigate that by marking your method with NoInlining (to prevent inlining)
 and NoOptimization (to prevent the JIT spitting out tail calls)[1], however,
 it is still possible for this to return incorrect results in certain other
 situations.
 
 
 http://msdn.microsoft.com/en-us/library/system.runtime.compilerservices.methodimploptions.aspx
 
  -Original Message-
  From: ozdotnet-boun...@ozdotnet.com [mailto:
 ozdotnet-boun...@ozdotnet.com] On Behalf Of David Kean
  Sent: Tuesday, March 22, 2011 10:06 PM
  To: ozDotNet
  Subject: RE: Raising property changed events
 
  Hmm, I'll check internally, but I'd be surprised if we give that
 guarantee. We're free to change our inlining policy at any time, in fact, we
 did just that in 3.5 SP1 x64 which broke a lot of customers who were relying
 on Assembly.GetExecutingAssembly() without explicitly turning off inlining
 for the method.
 
  Whether you can repro something now, is not a good indication of whether
 we'll continue to support in a future service pack or version - always check
  the docs. However, in saying that, the docs don't really make it clear that
 this might not work correctly in certain situations. In which case, if we
 don't give the above guarantee I'll make sure they call it out.
 
  -Original Message-
  From: ozdotnet-boun...@ozdotnet.com [mailto:
 ozdotnet-boun...@ozdotnet.com] On Behalf Of Mark Hurd
  Sent: Tuesday, March 22, 2011 9:36 PM
  To: ozDotNet
  Subject: Re: Raising property changed events
 
  On 23 March 2011 15:00, Mark Hurd markeh...@gmail.com wrote:
  I believe it was in this mailing list that we previously confirmed
  using GetCurrentMethod, even when included in convoluted ways,
  guarantees the method will not be inlined.
 
  Gmail says GetCurrentMethod has /not/ been mentioned before on this
 mailing list since I've been part of it, so I'm remembering that wrong.
 
  Can you show an example where GetCurrentMethod does not return the
  expected method?
 
  This request still stands however.
  --
  Regards,
  Mark Hurd, B.Sc.(Ma.)(Hons.)



RE: GetCurrentMethod possibly returning the wrong method (was Re: Raising property changed events)

2011-03-24 Thread David Kean
Okay, it turns out that I asked the wrong question to the JIT dev. 

I asked 'Does the JIT has any special knowledge about GetCurrentMethod when 
determining whether to inline a method?'. What I should have asked was 'Does 
the JIT have any special knowledge about *the characteristics* of a method like 
GetCurrentMethod when determining whether to inline a method?' The JIT guys are 
not managed developers - they talk in terms of CLI intrinsics not particular 
APIs.

The behavior you are seeing on that thread is correct. Callers of 
GetCurrentMethod will not be inlined. It is *not*, however, because of what 
Jeroen states. The JIT does not take into an account StackCrawlMark when 
determine whether to inline a caller, or the callee itself (it's not a 
coincidence that every method that gets a StackCrawlMark, is also marked as 
Noinlining). 

It's actually due to another reason; GetCurrentMethod is marked as 
RequireSecObject[1]. This method attribute (applied via the internal-only 
pseudo attribute, DynamicSecurityMethodAttribute) is used to indicate to the 
JIT that it should store extra information on the stack for methods that call 
it. It also has a side-effect of preventing those same methods from being 
inlined. 

One thing to note is that Assembly.GetCallingAssembly and 
Assembly.GetExecutingAssembly are not marked as RequireSecObject so these do 
not have the same guarantee. As the docs call out for 
Assembly.GetCallingAssembly you can't prevent your callers from being inlined 
(other than having opt out), however, the later, GetExecutingAssembly, you can 
apply NoInlining and NoOptimization to the method calling it to prevent it from 
getting the wrong results.

[1] http://msdn.microsoft.com/en-US/library/system.reflection.methodattributes



-Original Message-
From: ozdotnet-boun...@ozdotnet.com [mailto:ozdotnet-boun...@ozdotnet.com] On 
Behalf Of Mark Hurd
Sent: Thursday, March 24, 2011 5:20 PM
To: ozDotNet
Subject: Re: GetCurrentMethod possibly returning the wrong method (was Re: 
Raising property changed events)

Thanks for that clarification. However GoogleDesktop found my previous 
recollection discussing this was in the Microsoft NewsGroups, which now 
Googling for the subject of that exchange Reflection and compiler inlining 
found this other reply:

On 2009-12-11 6:18, Alex Clark wrote:
 Yes, because any method that calls MethodBase.GetCurrentMethod() will 
 not be inlined. This is because .GetCurrentMethod() includes a 
 StackCrawlMark, a special magical enum for methods that need to walk 
 the stack (like
 .GetCurrentMethod() predictably needs, to look for its caller) that 
 prevents the caller from being inlined. Trying to be clever by 
 sticking the call in a delegate like Mark did will not upset this, 
 because methods that call delegates are not inlined either.

(as retrieved from
http://www.eggheadcafe.com/software/aspnet/35469613/reflection-and-compiler-inlining.aspx
but there are a number of clones of this info)

So have things changed or was Alex Clark wrong?
--
Regards,
Mark Hurd, B.Sc.(Ma.)(Hons.)

On 25 March 2011 02:40, David Kean david.k...@microsoft.com wrote:
 I chased this up with one the devs on the JIT team. He confirmed that the 
 JIT/NGEN doesn't give this guarantee, both inlining and tail calls can cause 
 Assembly.GetExecutingAssembly, Assembly.GetCallingAssembly and 
 Method.GetCurrentMethod to return incorrect results. You can somewhat 
 mitigate that by marking your method with NoInlining (to prevent inlining) 
 and NoOptimization (to prevent the JIT spitting out tail calls)[1], however, 
 it is still possible for this to return incorrect results in certain other 
 situations.

 http://msdn.microsoft.com/en-us/library/system.runtime.compilerservice
 s.methodimploptions.aspx

 -Original Message-
 From: ozdotnet-boun...@ozdotnet.com 
 [mailto:ozdotnet-boun...@ozdotnet.com] On Behalf Of David Kean
 Sent: Tuesday, March 22, 2011 10:06 PM
 To: ozDotNet
 Subject: RE: Raising property changed events

 Hmm, I'll check internally, but I'd be surprised if we give that guarantee. 
 We're free to change our inlining policy at any time, in fact, we did just 
 that in 3.5 SP1 x64 which broke a lot of customers who were relying on 
 Assembly.GetExecutingAssembly() without explicitly turning off inlining for 
 the method.

 Whether you can repro something now, is not a good indication of whether 
 we'll continue to support in a future service pack or version - always check  
 the docs. However, in saying that, the docs don't really make it clear that 
 this might not work correctly in certain situations. In which case, if we 
 don't give the above guarantee I'll make sure they call it out.

 -Original Message-
 From: ozdotnet-boun...@ozdotnet.com 
 [mailto:ozdotnet-boun...@ozdotnet.com] On Behalf Of Mark Hurd
 Sent: Tuesday, March 22, 2011 9:36 PM
 To: ozDotNet
 Subject: Re: Raising property changed events

 On 23 March 2011 15:00, Mark Hurd markeh