You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by el...@apache.org on 2005/11/19 01:43:09 UTC
svn commit: r345612 - in
/directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common:
name/DnParser.java schema/DnComparator.java
Author: elecharny
Date: Fri Nov 18 16:42:21 2005
New Revision: 345612
URL: http://svn.apache.org/viewcvs?rev=345612&view=rev
Log:
Added a brutal and rude synchronization to avoid a race condition in DnParser usage.
At least, it corrects the problem, but the server is now 3 times slower.
Modified:
directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common/name/DnParser.java
directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common/schema/DnComparator.java
Modified: directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common/name/DnParser.java
URL: http://svn.apache.org/viewcvs/directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common/name/DnParser.java?rev=345612&r1=345611&r2=345612&view=diff
==============================================================================
--- directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common/name/DnParser.java (original)
+++ directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common/name/DnParser.java Fri Nov 18 16:42:21 2005
@@ -58,6 +58,7 @@
private ReusableAntlrValueLexer valueLexer;
+ private static final Object parserMutex = new Object();
/**
* Creates a regular non normalizing name parser.
@@ -67,6 +68,7 @@
public DnParser() throws NamingException
{
this.m_isNormalizing = false ;
+
try
{
init() ;
@@ -102,8 +104,12 @@
ne.setRootCause( e );
throw ne;
}
- this.m_isNormalizing = true ;
- this.m_parser.setNormalizer( a_normalizer ) ;
+
+ synchronized ( parserMutex )
+ {
+ this.m_isNormalizing = true ;
+ this.m_parser.setNormalizer( a_normalizer ) ;
+ }
}
@@ -125,26 +131,29 @@
*/
private void init() throws IOException
{
- this.m_selector = new TokenStreamSelector() ;
-
- // Create lexers and add them to the selector.
- typeLexer = new ReusableAntlrTypeLexer( new StringReader( "" ) );
- this.m_selector.addInputStream( typeLexer, ReusableAntlrTypeLexer.LEXER_KEY );
- valueLexer = new ReusableAntlrValueLexer( typeLexer.getInputState() );
- this.m_selector.addInputStream( valueLexer, ReusableAntlrValueLexer.LEXER_KEY );
-
- // Set selector on lexers, select initial lexer and initalize parser
- typeLexer.setSelector( this.m_selector ) ;
- valueLexer.setSelector( this.m_selector ) ;
- this.m_selector.select( ReusableAntlrTypeLexer.LEXER_KEY );
- this.m_parser = new ReusableAntlrNameParser( m_selector );
+ synchronized ( parserMutex )
+ {
+ this.m_selector = new TokenStreamSelector() ;
+
+ // Create lexers and add them to the selector.
+ typeLexer = new ReusableAntlrTypeLexer( new StringReader( "" ) );
+ this.m_selector.addInputStream( typeLexer, ReusableAntlrTypeLexer.LEXER_KEY );
+ valueLexer = new ReusableAntlrValueLexer( typeLexer.getInputState() );
+ this.m_selector.addInputStream( valueLexer, ReusableAntlrValueLexer.LEXER_KEY );
+
+ // Set selector on lexers, select initial lexer and initalize parser
+ typeLexer.setSelector( this.m_selector ) ;
+ valueLexer.setSelector( this.m_selector ) ;
+ this.m_selector.select( ReusableAntlrTypeLexer.LEXER_KEY );
+ this.m_parser = new ReusableAntlrNameParser( m_selector );
+ }
}
/**
* Resets the parser and lexers to be reused with new input
*/
- private synchronized void reset( String name )
+ private void reset( String name )
{
this.typeLexer.prepareNextInput( new StringReader( name + "#\n" ) );
this.valueLexer.prepareNextInput( typeLexer.getInputState() );
@@ -161,7 +170,7 @@
* @throws NamingException if a_name is invalid or the parsers plumbing
* breaks
*/
- public synchronized Name parse( String name, LdapName emptyName ) throws NamingException
+ public Name parse( String name, LdapName emptyName ) throws NamingException
{
if ( log.isDebugEnabled() )
{
@@ -178,13 +187,19 @@
{
if ( null == emptyName )
{
- reset( name );
- emptyName = new LdapName( m_parser.name() ) ;
+ synchronized ( parserMutex )
+ {
+ reset( name );
+ emptyName = new LdapName( m_parser.name() ) ;
+ }
}
else
{
- reset( name );
- emptyName.setList( m_parser.name() ) ;
+ synchronized ( parserMutex )
+ {
+ reset( name );
+ emptyName.setList( m_parser.name() ) ;
+ }
}
}
catch ( RecognitionException e )
Modified: directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common/schema/DnComparator.java
URL: http://svn.apache.org/viewcvs/directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common/schema/DnComparator.java?rev=345612&r1=345611&r2=345612&view=diff
==============================================================================
--- directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common/schema/DnComparator.java (original)
+++ directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common/schema/DnComparator.java Fri Nov 18 16:42:21 2005
@@ -41,6 +41,8 @@
/** The parser used to parse DN Strings */
private NameParser parser ;
+ private static final Object parserMutex = new Object();
+
// ------------------------------------------------------------------------
// C O N S T R U C T O R S
@@ -53,7 +55,10 @@
*/
public DnComparator() throws NamingException
{
- parser = new DnParser() ;
+ synchronized ( parserMutex )
+ {
+ parser = new DnParser() ;
+ }
}
@@ -64,7 +69,10 @@
*/
public DnComparator( NameComponentNormalizer normalizer ) throws NamingException
{
- parser = new DnParser( normalizer ) ;
+ synchronized ( parserMutex )
+ {
+ parser = new DnParser( normalizer ) ;
+ }
}
@@ -97,19 +105,22 @@
{
try
{
- dn1 = parser.parse( ( String ) obj1 ) ;
+ synchronized ( parserMutex )
+ {
+ dn1 = parser.parse( ( String ) obj1 ) ;
+ }
}
catch ( NamingException ne )
{
throw new IllegalArgumentException(
- "first argument was not a distinguished name" ) ;
+ "first argument (" + obj1 + ") was not a distinguished name" ) ;
}
}
}
else
{
throw new IllegalArgumentException(
- "first argument was not a distinguished name" ) ;
+ "first argument (" + obj1 + ") was not a Name or a String" ) ;
}
// Figure out how to compose the Name for the second object
@@ -121,7 +132,10 @@
{
try
{
- dn2 = parser.parse( ( String ) obj2 ) ;
+ synchronized ( parserMutex )
+ {
+ dn2 = parser.parse( ( String ) obj2 ) ;
+ }
}
catch ( NamingException ne )
{
Performances again ...
Posted by Emmanuel Lecharny <el...@apache.org>.
After having switched to Twix codec, I got a drastic speed improvment :
1 thread, 2000 bind, search, unbind :
* August 30 benchmark : 120.5 req/s
* November 19 benchmark with Snickers : 107 req/s
* November 19 benchmark with Twix : 151 req/s
* August 30 OpenLdap benchmark : 165 req/s
5 threads, 400 bind, search, unbind :
* August 30 benchmark : 143.2 req/s
* November 19 benchmark with Snickers : 109 req/s
* November 19 benchmark with Twix : 172 req/s
* August 30 OpenLdap benchmark : 181 req/s
So we actually get a 20 % speed improvment, and we are closing the gap
with OpenLdap.
WARNING : this is not a valid benchmark of ApacheDS !!! This is just a
way to check that the server behaves correctly when using multi-users.
The collected numbers are also a good way to validate that the
improvments are real.
--Emmanuel.
On Sat, 2005-11-19 at 10:18 +0100, Emmanuel Lecharny wrote:
> > >Added a brutal and rude synchronization to avoid a race condition in DnParser usage.
> > >
> > >At least, it corrects the problem, but the server is now 3 times slower.
> > >
> > >
> > Darn I was enjoying the recent speed ups too. We need to figure this
> > one out.
>
> Well, after having tuned the log level, the penalty is not that awfull :
> something like 30%.
>
> I got 113 search requests per second, against 143 req/s four month ago.
>
> There are huge areas of optimization to work on, but from now one, I bet
> the first move is to finalize the 1.0 beta. We don't need a blazing fast
> server which is full of bugs ;)
>
> -- Emmanuel
>
> ---------------------------------------------------------------------------------------
> Wanadoo vous informe que cet e-mail a ete controle par l'anti-virus mail.
> Aucun virus connu a ce jour par nos services n'a ete detecte.
>
>
>
Re: svn commit: r345612 - in /directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common:
name/DnParser.java schema/DnComparator.java
Posted by Alex Karasulu <ao...@bellsouth.net>.
Trustin Lee wrote:
> 2005/11/21, Emmanuel Lecharny <elecharny@gmail.com
> <ma...@gmail.com>>:
>
> On Mon, 2005-11-21 at 19:41 +0900, Trustin Lee wrote:
> > Why don't we use pooling techinique? We could use
> commons-pooling or
> > our proprietary stack implementation. :)
> >
> > Trustin
> >
>
> This is an option, but this is complicated. The best solution
> could be
> to have a stateless parser which will be totally threadsafe.
>
>
> I agree. If we can implement a stateless parser, that's the best.
>
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
I agree regarding the use of a totally stateless DNParser it will be
very fast and no synchronization;
pooling will still have synchronization.
Alex
Re: svn commit: r345612 - in /directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common: name/DnParser.java schema/DnComparator.java
Posted by Trustin Lee <tr...@gmail.com>.
2005/11/21, Emmanuel Lecharny <el...@gmail.com>:
>
> On Mon, 2005-11-21 at 19:41 +0900, Trustin Lee wrote:
> > Why don't we use pooling techinique? We could use commons-pooling or
> > our proprietary stack implementation. :)
> >
> > Trustin
> >
>
> This is an option, but this is complicated. The best solution could be
> to have a stateless parser which will be totally threadsafe.
I agree. If we can implement a stateless parser, that's the best.
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
Re: svn commit: r345612 - in
/directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common:
name/DnParser.java schema/DnComparator.java
Posted by Emmanuel Lecharny <el...@gmail.com>.
On Mon, 2005-11-21 at 19:41 +0900, Trustin Lee wrote:
> Why don't we use pooling techinique? We could use commons-pooling or
> our proprietary stack implementation. :)
>
> Trustin
>
This is an option, but this is complicated. The best solution could be
to have a stateless parser which will be totally threadsafe.
--Emmanuel
Re: svn commit: r345612 - in /directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common: name/DnParser.java schema/DnComparator.java
Posted by Trustin Lee <tr...@gmail.com>.
2005/11/22, Alex Karasulu <ao...@bellsouth.net>:
>
> Trustin Lee wrote:
>
> > Why don't we use pooling techinique? We could use commons-pooling or
> > our proprietary stack implementation. :)
> >
> What you talkin bout Willis? :) Proprietary?
I mean 'our own'. Sorry for bad English!
> 2005/11/19, Emmanuel Lecharny < elecharny@apache.org
> > <ma...@apache.org>>:
> >
> >
> > > >Added a brutal and rude synchronization to avoid a race
> > condition in DnParser usage.
> > > >
> > > >At least, it corrects the problem, but the server is now 3
> > times slower.
> > > >
> > > >
> > > Darn I was enjoying the recent speed ups too. We need to figure
> > this
> > > one out.
> >
> > Well, after having tuned the log level, the penalty is not that
> > awfull :
> > something like 30%.
> >
> > I got 113 search requests per second, against 143 req/s four month
> > ago.
> >
> > There are huge areas of optimization to work on, but from now one,
> > I bet
> > the first move is to finalize the 1.0 beta. We don't need a
> > blazing fast
> > server which is full of bugs ;)
> >
> +1. I think we should add features up until we get to a beta. Then we
> can spend our time optimizing. Who knows what will happen to your
> optimizations as new features are stacked on. We might break them I
> mean and you will have spent good time on it. Then again I am enjoying
> the performance boosts we have gotten from your recent work. Thanks!
>
> Alex
>
>
--
what we call human nature is actually human habit
--
http://gleamynode.net/
Re: svn commit: r345612 - in /directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common:
name/DnParser.java schema/DnComparator.java
Posted by Alex Karasulu <ao...@bellsouth.net>.
Trustin Lee wrote:
> Why don't we use pooling techinique? We could use commons-pooling or
> our proprietary stack implementation. :)
>
What you talkin bout Willis? :) Proprietary?
> 2005/11/19, Emmanuel Lecharny < elecharny@apache.org
> <ma...@apache.org>>:
>
>
> > >Added a brutal and rude synchronization to avoid a race
> condition in DnParser usage.
> > >
> > >At least, it corrects the problem, but the server is now 3
> times slower.
> > >
> > >
> > Darn I was enjoying the recent speed ups too. We need to figure
> this
> > one out.
>
> Well, after having tuned the log level, the penalty is not that
> awfull :
> something like 30%.
>
> I got 113 search requests per second, against 143 req/s four month
> ago.
>
> There are huge areas of optimization to work on, but from now one,
> I bet
> the first move is to finalize the 1.0 beta. We don't need a
> blazing fast
> server which is full of bugs ;)
>
+1. I think we should add features up until we get to a beta. Then we
can spend our time optimizing. Who knows what will happen to your
optimizations as new features are stacked on. We might break them I
mean and you will have spent good time on it. Then again I am enjoying
the performance boosts we have gotten from your recent work. Thanks!
Alex
Re: svn commit: r345612 - in /directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common: name/DnParser.java schema/DnComparator.java
Posted by Trustin Lee <tr...@gmail.com>.
Why don't we use pooling techinique? We could use commons-pooling or our
proprietary stack implementation. :)
Trustin
2005/11/19, Emmanuel Lecharny <el...@apache.org>:
>
>
> > >Added a brutal and rude synchronization to avoid a race condition in
> DnParser usage.
> > >
> > >At least, it corrects the problem, but the server is now 3 times
> slower.
> > >
> > >
> > Darn I was enjoying the recent speed ups too. We need to figure this
> > one out.
>
> Well, after having tuned the log level, the penalty is not that awfull :
> something like 30%.
>
> I got 113 search requests per second, against 143 req/s four month ago.
>
> There are huge areas of optimization to work on, but from now one, I bet
> the first move is to finalize the 1.0 beta. We don't need a blazing fast
> server which is full of bugs ;)
>
> -- Emmanuel
>
>
--
what we call human nature is actually human habit
--
http://gleamynode.net/
Re: svn commit: r345612 - in
/directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common:
name/DnParser.java schema/DnComparator.java
Posted by Emmanuel Lecharny <el...@apache.org>.
> >Added a brutal and rude synchronization to avoid a race condition in DnParser usage.
> >
> >At least, it corrects the problem, but the server is now 3 times slower.
> >
> >
> Darn I was enjoying the recent speed ups too. We need to figure this
> one out.
Well, after having tuned the log level, the penalty is not that awfull :
something like 30%.
I got 113 search requests per second, against 143 req/s four month ago.
There are huge areas of optimization to work on, but from now one, I bet
the first move is to finalize the 1.0 beta. We don't need a blazing fast
server which is full of bugs ;)
-- Emmanuel
Re: svn commit: r345612 - in /directory/shared/ldap/trunk/common/src/main/java/org/apache/ldap/common:
name/DnParser.java schema/DnComparator.java
Posted by Alex Karasulu <ao...@bellsouth.net>.
elecharny@apache.org wrote:
>Author: elecharny
>Date: Fri Nov 18 16:42:21 2005
>New Revision: 345612
>
>URL: http://svn.apache.org/viewcvs?rev=345612&view=rev
>Log:
>Added a brutal and rude synchronization to avoid a race condition in DnParser usage.
>
>At least, it corrects the problem, but the server is now 3 times slower.
>
>
Darn I was enjoying the recent speed ups too. We need to figure this
one out. Again this is one of those problems that pervades every part
of the server from its database up to it's protocol handlers.
Alex