cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael McKibben <mmckib...@ncube.com>
Subject RE: [C2] sitemap creation threading issue ?
Date Wed, 22 Aug 2001 20:40:46 GMT
Sorry for this thread getting off topic of cocoon... If anyone wants to jump
in on this, we can continue off list. Thanks.

Yes, I believe your example would work. The issue is how the JVM reorders
the bytecodes during optimization. In the first example, the bytecode for
the assignment looks something like this:

new <singleton class constant pool index>
dup
invokespecial <singleton class constructor constant pool index>
putstatic <singleton instance field constant pool index>

By first assigning the new instance to a local variable, you get something
like:

new <singleton class constant pool index>
dup
invokespecial <singleton class constructor constant pool index>
astore_xxx <xxx is stack offset for local variable>
aload_xxx <xxx is stack offset for local variable>
putstatic <singleton instance field constant pool index>

There should be no possibility for another thread in the second example to
receive false negative on "the_instance == null" test. But perhaps I still
miss some real obscure JVM issue? It was my understanding that the JVM must
validate all objects on the heap and that a non-initialized object was
impossible. The article mentioned earlier implies that this is not so. Now
if I understand the article correctly, something like this was happening:

Thread1: (inside synchronized block)
new <singleton class constant pool index>
dup
putstatic <singleton instance field constant pool index> (assignment done
before object initialization!!!)

Thread2: (unsynchronized test for null on static singleton member)
getstatic <singleton instance field constant pool index>
ifnonnull <goto some line number>
... T2 uses uninitialized singleton object!

Thread1:
(invokespecial deferred till later due to JVM optimization)

The article did not mention what happens when a thread tries to use an
uninitialized object. I would guess a VerifyError or some other Error is
thrown by the JVM. In any case, pretty interesting stuff. Regards,

--mike

-----Original Message-----
From: Ovidiu Predescu [mailto:ovidiu@cup.hp.com]
Sent: Tuesday, August 21, 2001 4:15 PM
To: Michael McKibben
Cc: 'cocoon-dev@xml.apache.org'
Subject: Re: [C2] sitemap creation threading issue ? 


Or you can simplify it even more, like this:

class singleton
{
  private static singleton the_instance;

  private singleton()
  {
    // Bunches of reads and writes that initialize
    // the object occur here.
  }

  public static singleton get_instance()
  {
    // This is the method where the problem is! It does NOT work!
    if (the_instance == null) 
      synchronized (singleton.class) {
        if (the_instance == null) {
          singleton v = new singleton();
          the_instance = v;
        }
      }
    return the_instance; 
  }
}

How about it?

Greetings,
Ovidiu

On Mon, 20 Aug 2001 11:26:51 -0600, Michael McKibben <mmckibben@ncube.com>
wrote:

> I read the article you referred to. It seems to me that this warning (i.e.
> the possibility of returning a pointer to an uninitialized object) does
not
> apply to the case where you are assigning to a local (i.e.. stack)
variable
> and not some instance variable (i.e. shared memory) and putting that value
> in a Map. The example in the article was testing/assigning a static member
> variable that could be pointing to an uninitialized object. There is no
> possibility of this happening if the test is made on a local stack
variable.
> Am I incorrect in this interpretation? Or do I still miss something? 
> 
> For example if the code in the article had been changed to the following,
> there would have been no issue:
> 
> 
>     public static Disfunctional_singleton get_instance() 
>     { 
> 	Disfunctional_singleton instance = the_instance;
> 	if( instance == null ) 
>             synchronized( Disfunctional_singleton.class ) 
>             { 
>                    instance = the_instance;		 
> 	       if(instance == null ) {
>                     instance = new Disfunctional_singleton(); 
>                     new_instance = instance;
>                    }
>             } 
>         return instance; 
>     } 
> 
> In any case, I am rather shocked by the analysis! I am going to have to
look
> through all my singleton code now! But I don't think that the "double
check"
> lock pattern should be thrown away entirely. Thanks for the info. Regards,
> 
> --mike
> 
> -----Original Message-----
> From: Vadim Gritsenko [mailto:vadim.gritsenko@ionidea.com]
> Sent: Monday, August 20, 2001 10:28 AM
> To: cocoon-dev@xml.apache.org
> Subject: RE: [C2] sitemap creation threading issue ?
> 
> 
> According to
> http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-toolbox.html
> this technique does not work, especially on SMP systems.
> 
> Vadim
> 
> > -----Original Message-----
> > From: Michael McKibben [mailto:mmckibben@ncube.com]
> > Sent: Monday, August 20, 2001 12:20 PM
> > To: 'cocoon-dev@xml.apache.org'
> > Subject: RE: [C2] sitemap creation threading issue ?
> > 
> > 
> > I briefly glanced at the patch and one thing you can do to optimize the
> > syncronization is to apply the "double check" lock pattern. This will
> allow
> > you to remove the synchronized keyword from the method.  For example:
> > 
> > Handler sitemapHandler = (Handler)sitemaps.get(source);
> > if (sitemapHandler != null) {
> > ...
> > }
> > 
> > Remove synchronized from the method and change body to:
> > 
> > Handler sitemapHandler = (Handler)sitemaps.get(source);
> > if (sitemapHandler == null) {
> > 	synchronized (sitemaps) {
> > 		sitemapHandler = (Handler)sitemaps.get(source);
> > 		if (sitemapHandler == null) {
> > 			// create handler, put in the Map ...
> > 			sitemaps.put(...)
> > 		}
> > 	}
> > }
> > 
> > Notice that threads only get synchronized in the case where a new
handler
> > has not yet been created, not every call into the method!
> > 
> > Regards,
> > 
> > --mike
> > 
> > -----Original Message-----
> > From: Morrison, John [mailto:John.Morrison@uk.experian.com]
> > Sent: Monday, August 20, 2001 5:47 AM
> > To: 'cocoon-dev@xml.apache.org'
> > Subject: RE: [C2] sitemap creation threading issue ?
> > 
> > 
> > Hi all, esp Marcus,
> > 
> > I too have been running Load tests, I also noticed that 'something' was
> > happening regularly to slow the response.  I hadn't gotten round to
> checking
> > the code though (the testing machines are isolated - 0 access from/to my
> dev
> > station).
> > 
> > I saw your patch and rejoiced ;) but it's taken the response of some of
my
> > pages from a couple of hundred milliseconds to over 5!  I've not yet
> looked
> > at _what_ your patch did, but have you seen a similar increase in time?
> > 
> > J.
> > 
> > > -----Original Message-----
> > > From: Carsten Ziegeler [mailto:cziegeler@sundn.de]
> > > Sent: Monday, 20 August 2001 7:57 am
> > > To: cocoon-dev@xml.apache.org
> > > Subject: AW: [C2] sitemap creation threading issue ?
> > > 
> > > 
> > > Hi Marcus,
> > > 
> > > thanks for your patch. I applied it, please cross check :-)
> > > 
> > > Carsten
> > > 
> > > > -----Ursprungliche Nachricht-----
> > > > Von: Marcus Crafter [mailto:crafterm@fztig938.bank.dresdner.net]
> > > > Gesendet: Samstag, 18. August 2001 22:41
> > > > An: Cocoon Developers Mailing List
> > > > Betreff: [C2] sitemap creation threading issue ?
> > > > 
> > > > 
> > > > Hi *,
> > > > 
> > > > 	Hope all is well.
> > > > 
> > > > 	Michael and I have spent the day testing our C2 application
with
> > > > 	LoadRunner and have potentially uncovered a threading 
> > > problem during
> > > > 	sitemap creation.
> > > > 
> > > > 	We're not experts with the code but from our understanding
the
> > > > 	following is happening, please let us know if we are 
> > > right/wrong:
> > > > 
> > > > 	There seems to be a problem with the getHandler() 
> > > method, located in
> > > > 	the sitemap Manager class (line 154). getHandler() attempts 
> > > > to access a
> > > > 	sitemap handler object for each request for processing. If 
> > > > the handler
> > > > 	object is not available it creates one, causing the 
> > > sitemap to be
> > > > 	generated.
> > > > 
> > > > 	We've noticed under load, that many handler objects are 
> > > created for
> > > > 	the same sitemap. This is because getHandler() does not 
> > > protect the
> > > > 	following lines:
> > > > 
> > > > 		Handler sitemapHandler =
(Handler)sitemaps.get(source);
> > > > 
> > > > 	and
> > > > 
> > > > 		sitemaps.put(source, sitemapHandler);
> > > > 
> > > > 	as a critical area.
> > > > 
> > > > 	If multiple concurrent threads pass through 
> > > getHandler() which are
> > > > 	requests for resources from the same sitemap, the first line
> > > > 	above will return null multiple times causing the same 
> > > sitemap to be
> > > > 	compiled several times, each by individual Handler objects.
> > > > 	
> > > > 	This happens because sitemaps.put() executes after each 
> > > > sitemap handler
> > > > 	object is created (which can take time for large sitemaps), 
> > > > and cannot
> > > > 	prevent other incoming threads from waiting until it 
> > > adds the newly
> > > > 	created handler object into the 'sitemaps' hashmap.
> > > > 
> > > > 	When we synchronized the getHandler method to protect the
> > > > 	getting/setting of the sitemaps hashmap, we saw that the
sitemap
> > > > 	handler object was created only once, and that the
application
> > > > 	performed much better under load. Previously the same 
> > > > sitemap handler
> > > > 	object was created as many times as we had simultaneous
threads
> > > > 	make requests.
> > > > 
> > > > 	Attached is a diff of the change we made. There might 
> > > be a better
> > > > 	solution as the Handler class seems to be built to 
> > > handle this, it's
> > > > 	just that the allocation of a new Handler objects per 
> > > > sitemap, defeats
> > > > 	it's internal multi-thread logic.
> > > > 
> > > > 	Any comments/thoughts/suggestions ?
> > > > 
> > > > 	Cheers,
> > > > 
> > > > 	Marcus
> > > > 
> > > > -- 
> > > >         .....
> > > >      ,,$$$$$$$$$,      Marcus Crafter
> > > >     ;$'      '$$$$:    Computer Systems Engineer
> > > >     $:         $$$$:   Open Software Associates GmbH
> > > >      $       o_)$$$:   82-84 Mainzer Landstrasse
> > > >      ;$,    _/\ &&:'   60327 Frankfurt Germany
> > > >        '     /( &&&
> > > >            \_&&&&'     Email : Marcus.Crafter@osa.de
> > > >           &&&&.        Business Hours : +49 69 9757 200
> > > >     &&&&&&&:
> > > > 
> > > 
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
> > > For additional commands, email: cocoon-dev-help@xml.apache.org
> > > 
> > 
> > 
> > =======================================================================
> > Information in this email and any attachments are confidential, and may
> > not be copied or used by anyone other than the addressee, nor disclosed
> > to any third party without our permission.  There is no intention to
> > create any legally binding contract or other commitment through the use
> > of this email.
> > 
> > Experian Limited (registration number 653331).  
> > Registered office: Talbot House, Talbot Street, Nottingham NG1 5HF
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
> > For additional commands, email: cocoon-dev-help@xml.apache.org
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
> > For additional commands, email: cocoon-dev-help@xml.apache.org
> > 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
> For additional commands, email: cocoon-dev-help@xml.apache.org
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
> For additional commands, email: cocoon-dev-help@xml.apache.org
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org

Mime
View raw message