You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-dev@axis.apache.org by "Elwin Ho (JIRA)" <ax...@ws.apache.org> on 2006/12/09 00:59:20 UTC

[jira] Created: (AXIS-2597) CLONE -SimpleUUIDGen.nextUUID() is very, very, very slow

CLONE -SimpleUUIDGen.nextUUID() is very, very, very slow
--------------------------------------------------------

                 Key: AXIS-2597
                 URL: http://issues.apache.org/jira/browse/AXIS-2597
             Project: Apache Axis
          Issue Type: Bug
    Affects Versions: 1.1
         Environment: Any
            Reporter: Elwin Ho


It takes .4917 seconds to generate just one UUID.  (Yes, seconds.)  This renders the feature very nearly useless.

In SimpleUUIDGen, there is a simple, but very expensive, bug..  It creates a new instance of SecureRandom on each call to nextUUID().  Making a new instance of SecureRandom is very expensive because it has to set up the environment for producing cryptographically strong random numbers.

The instance of SecureRandom should be in a static variable and init'ed just once.  Then, you call getNextLong() when you want another number from it.  It would be much faster that way without compromising the integrity of the UUIDs generated.

BTW, If you look at the comment I copied from the code, it appears that this is what the developer meant to make the SecureRandom instance static, but forgot.

    /**
     * Creates a new UUID. The algorithm used is described by The Open Group.
     * See <a href="http://www.opengroup.org/onlinepubs/009629399/apdxa.htm">
     * Universal Unique Identifier</a> for more details.
     * <p>
     * Due to a lack of functionality in Java, a part of the UUID is a secure
     * random. This results in a long processing time when this method is called
     * for the first time.
     */


Here's a proposed fix.  In the member variables, add the following.

    private static Random secureRandom = null;
    static {
        try {
            secureRandom = SecureRandom.getInstance("SHA1PRNG", "SUN");
        } catch (Exception e) {
            secureRandom = new Random();
        }
    }


The, remove the code near line 235 where the SecureRandom is being created.  

This change should do it just once.




-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


[jira] Commented: (AXIS-2597) CLONE -SimpleUUIDGen.nextUUID() is very, very, very slow

Posted by "Dug (JIRA)" <ax...@ws.apache.org>.
    [ https://issues.apache.org/jira/browse/AXIS-2597?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12489876 ] 

Dug commented on AXIS-2597:
---------------------------

Take a look at the UUIDGenerator in the org.apache.axis.wsa package - I think it works better and we should have just one anyway.

> CLONE -SimpleUUIDGen.nextUUID() is very, very, very slow
> --------------------------------------------------------
>
>                 Key: AXIS-2597
>                 URL: https://issues.apache.org/jira/browse/AXIS-2597
>             Project: Axis
>          Issue Type: Bug
>    Affects Versions: 1.1
>         Environment: Any
>            Reporter: Elwin Ho
>
> It takes .4917 seconds to generate just one UUID.  (Yes, seconds.)  This renders the feature very nearly useless.
> In SimpleUUIDGen, there is a simple, but very expensive, bug..  It creates a new instance of SecureRandom on each call to nextUUID().  Making a new instance of SecureRandom is very expensive because it has to set up the environment for producing cryptographically strong random numbers.
> The instance of SecureRandom should be in a static variable and init'ed just once.  Then, you call getNextLong() when you want another number from it.  It would be much faster that way without compromising the integrity of the UUIDs generated.
> BTW, If you look at the comment I copied from the code, it appears that this is what the developer meant to make the SecureRandom instance static, but forgot.
>     /**
>      * Creates a new UUID. The algorithm used is described by The Open Group.
>      * See <a href="http://www.opengroup.org/onlinepubs/009629399/apdxa.htm">
>      * Universal Unique Identifier</a> for more details.
>      * <p>
>      * Due to a lack of functionality in Java, a part of the UUID is a secure
>      * random. This results in a long processing time when this method is called
>      * for the first time.
>      */
> Here's a proposed fix.  In the member variables, add the following.
>     private static Random secureRandom = null;
>     static {
>         try {
>             secureRandom = SecureRandom.getInstance("SHA1PRNG", "SUN");
>         } catch (Exception e) {
>             secureRandom = new Random();
>         }
>     }
> The, remove the code near line 235 where the SecureRandom is being created.  
> This change should do it just once.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


[jira] Updated: (AXIS-2597) CLONE -SimpleUUIDGen.nextUUID() is very, very, very slow

Posted by "Rodrigo Ruiz (JIRA)" <ax...@ws.apache.org>.
     [ https://issues.apache.org/jira/browse/AXIS-2597?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Rodrigo Ruiz updated AXIS-2597:
-------------------------------

    Attachment: SimpleUUIDGen.diff

Elwin, it seems you are wrong. From the code snippet you show in the description, the variable is already declared static, and initialized in a static block. This means it is only created once per class-loader.

I think the UUID standard comprises several algorithms for building the identifiers. This would explain why there are more than one generator.

Most of the generation time in SimpleUUIDGen is spent on random value generation. This is because it uses a SecureRandom instance, which is rather slow.

I believe this class must not be changed too much, as the algorithm must be kept standard compliant.

If you do not require secure random generation, you may better use the default one (FastUUIDGen, in the same package).

I have tried to improve the execution time by moving some of the code to the static block, and optimizing some of the private methods. The times changed from 16000ms to 12000ms for 100 ids. This means an improvement of around 25%. I attach the patch for these changes.

> CLONE -SimpleUUIDGen.nextUUID() is very, very, very slow
> --------------------------------------------------------
>
>                 Key: AXIS-2597
>                 URL: https://issues.apache.org/jira/browse/AXIS-2597
>             Project: Axis
>          Issue Type: Bug
>    Affects Versions: 1.1
>         Environment: Any
>            Reporter: Elwin Ho
>         Attachments: SimpleUUIDGen.diff
>
>
> It takes .4917 seconds to generate just one UUID.  (Yes, seconds.)  This renders the feature very nearly useless.
> In SimpleUUIDGen, there is a simple, but very expensive, bug..  It creates a new instance of SecureRandom on each call to nextUUID().  Making a new instance of SecureRandom is very expensive because it has to set up the environment for producing cryptographically strong random numbers.
> The instance of SecureRandom should be in a static variable and init'ed just once.  Then, you call getNextLong() when you want another number from it.  It would be much faster that way without compromising the integrity of the UUIDs generated.
> BTW, If you look at the comment I copied from the code, it appears that this is what the developer meant to make the SecureRandom instance static, but forgot.
>     /**
>      * Creates a new UUID. The algorithm used is described by The Open Group.
>      * See <a href="http://www.opengroup.org/onlinepubs/009629399/apdxa.htm">
>      * Universal Unique Identifier</a> for more details.
>      * <p>
>      * Due to a lack of functionality in Java, a part of the UUID is a secure
>      * random. This results in a long processing time when this method is called
>      * for the first time.
>      */
> Here's a proposed fix.  In the member variables, add the following.
>     private static Random secureRandom = null;
>     static {
>         try {
>             secureRandom = SecureRandom.getInstance("SHA1PRNG", "SUN");
>         } catch (Exception e) {
>             secureRandom = new Random();
>         }
>     }
> The, remove the code near line 235 where the SecureRandom is being created.  
> This change should do it just once.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


[jira] Commented: (AXIS-2597) CLONE -SimpleUUIDGen.nextUUID() is very, very, very slow

Posted by "Elwin Ho (JIRA)" <ax...@ws.apache.org>.
    [ http://issues.apache.org/jira/browse/AXIS-2597?page=comments#action_12457017 ] 
            
Elwin Ho commented on AXIS-2597:
--------------------------------

It takes around 1500 ms to generate 10 ids. It is way to slow to use compare to   (java.rmi.server.UID only take 3 ms for 10 ids) 
Env.
PM 1.8 hz. 
Java 1.4
2G memory.



    long elwin_startTime = System.currentTimeMillis();

        for (int i=0; i<10; i ++)
        {
            suuidg.nextUUID();
        }
        System.out.println("take "
                                          + ((System.currentTimeMillis()-elwin_startTime))
                                          + " ms");
        
        

> CLONE -SimpleUUIDGen.nextUUID() is very, very, very slow
> --------------------------------------------------------
>
>                 Key: AXIS-2597
>                 URL: http://issues.apache.org/jira/browse/AXIS-2597
>             Project: Apache Axis
>          Issue Type: Bug
>    Affects Versions: 1.1
>         Environment: Any
>            Reporter: Elwin Ho
>
> It takes .4917 seconds to generate just one UUID.  (Yes, seconds.)  This renders the feature very nearly useless.
> In SimpleUUIDGen, there is a simple, but very expensive, bug..  It creates a new instance of SecureRandom on each call to nextUUID().  Making a new instance of SecureRandom is very expensive because it has to set up the environment for producing cryptographically strong random numbers.
> The instance of SecureRandom should be in a static variable and init'ed just once.  Then, you call getNextLong() when you want another number from it.  It would be much faster that way without compromising the integrity of the UUIDs generated.
> BTW, If you look at the comment I copied from the code, it appears that this is what the developer meant to make the SecureRandom instance static, but forgot.
>     /**
>      * Creates a new UUID. The algorithm used is described by The Open Group.
>      * See <a href="http://www.opengroup.org/onlinepubs/009629399/apdxa.htm">
>      * Universal Unique Identifier</a> for more details.
>      * <p>
>      * Due to a lack of functionality in Java, a part of the UUID is a secure
>      * random. This results in a long processing time when this method is called
>      * for the first time.
>      */
> Here's a proposed fix.  In the member variables, add the following.
>     private static Random secureRandom = null;
>     static {
>         try {
>             secureRandom = SecureRandom.getInstance("SHA1PRNG", "SUN");
>         } catch (Exception e) {
>             secureRandom = new Random();
>         }
>     }
> The, remove the code near line 235 where the SecureRandom is being created.  
> This change should do it just once.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


[jira] Commented: (AXIS-2597) CLONE -SimpleUUIDGen.nextUUID() is very, very, very slow

Posted by "Nathan Wang (JIRA)" <ax...@ws.apache.org>.
    [ http://issues.apache.org/jira/browse/AXIS-2597?page=comments#action_12457021 ] 
            
Nathan Wang commented on AXIS-2597:
-----------------------------------


And this is using 1.4 release in java version. 

> CLONE -SimpleUUIDGen.nextUUID() is very, very, very slow
> --------------------------------------------------------
>
>                 Key: AXIS-2597
>                 URL: http://issues.apache.org/jira/browse/AXIS-2597
>             Project: Apache Axis
>          Issue Type: Bug
>    Affects Versions: 1.1
>         Environment: Any
>            Reporter: Elwin Ho
>
> It takes .4917 seconds to generate just one UUID.  (Yes, seconds.)  This renders the feature very nearly useless.
> In SimpleUUIDGen, there is a simple, but very expensive, bug..  It creates a new instance of SecureRandom on each call to nextUUID().  Making a new instance of SecureRandom is very expensive because it has to set up the environment for producing cryptographically strong random numbers.
> The instance of SecureRandom should be in a static variable and init'ed just once.  Then, you call getNextLong() when you want another number from it.  It would be much faster that way without compromising the integrity of the UUIDs generated.
> BTW, If you look at the comment I copied from the code, it appears that this is what the developer meant to make the SecureRandom instance static, but forgot.
>     /**
>      * Creates a new UUID. The algorithm used is described by The Open Group.
>      * See <a href="http://www.opengroup.org/onlinepubs/009629399/apdxa.htm">
>      * Universal Unique Identifier</a> for more details.
>      * <p>
>      * Due to a lack of functionality in Java, a part of the UUID is a secure
>      * random. This results in a long processing time when this method is called
>      * for the first time.
>      */
> Here's a proposed fix.  In the member variables, add the following.
>     private static Random secureRandom = null;
>     static {
>         try {
>             secureRandom = SecureRandom.getInstance("SHA1PRNG", "SUN");
>         } catch (Exception e) {
>             secureRandom = new Random();
>         }
>     }
> The, remove the code near line 235 where the SecureRandom is being created.  
> This change should do it just once.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org