Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 323455

Summary: SocketUtil.isLocalhost() method takes a long time
Product: [WebTools] WTP ServerTools Reporter: Rajiv Senthilnathan <rajiv.senthilnathan>
Component: wst.serverAssignee: Elson Yuen <eyuen7>
Status: RESOLVED FIXED QA Contact: Angel Vera <arvera>
Severity: normal    
Priority: P3 CC: arvera, david_williams, eyuen7
Version: unspecifiedFlags: david_williams: pmc_approved+
arvera: pmc_approved? (raghunathan.srinivasan)
arvera: pmc_approved? (naci.dai)
arvera: pmc_approved? (deboer)
arvera: pmc_approved? (neil.hauge)
arvera: pmc_approved? (kaloyan)
arvera: review+
Target Milestone: 3.2.2   
Hardware: All   
OS: All   
See Also: https://git.eclipse.org/r/108988
Whiteboard: PMC_approved
Attachments:
Description Flags
Proposed Patch
none
v1.0 arvera: iplog+, arvera: review+

Description Rajiv Senthilnathan CLA 2010-08-23 23:06:08 EDT 
Build Identifier: latest in CVS as of August 23, 2010

The method is called many times and can cause substantial slow-downs, especially when network conditions are poor.

The efficiency of this method can be improved by doing the following:

1) Making the cache stay alive even once the method is exited
2) Check the caches at the beginning of the method (before the cache is checked to be old) and later in the method (after the cache has been refreshed)
3) Call InetAddress.getLocalHost() only once and reuse the computed value since this is an expensive method to call
4) Add the host to the notLocalHostCache if we are returning false at the end of the method

Reproducible: Always

Steps to Reproduce:
1. Start the New Server creation wizard
2. Change the server's host name.
3. Proceed through the wizard.

When you debug you can see that the isLocalhost method is called many times even for a single action, thereby increasing the time spent in the method (often doing work that has already been done).
Comment 1 Rajiv Senthilnathan CLA 2010-08-23 23:13:27 EDT 
Created attachment 177280 [details]
Proposed Patch

Proposed patch for Bug 323455 - SocketUtil.isLocalhost() method takes a long time

See bug defect description for details on what the patch does.
Comment 2 Elson Yuen CLA 2010-08-24 18:02:36 EDT 
Please assign this one to me. There are a couple of things that needs to be changed on the patch. I'll create a new patch for it:

1. a couple of accessing and updating on the cache objects are not within the synchronized of the lock object that may causes problems.
2. InetAddress.getLocalHost() is being called more than once within the method and should be using the cached value to make the method perform better.
3. After the check on:
			if (host.equals(localHostaddr.getHostName().toLowerCase())
					|| host.equals(localHostaddr.getCanonicalHostName().toLowerCase())
					|| host.equals(localHostaddr.getHostAddress().toLowerCase()))
				return true;

The host should be added to the localHostCache to cache the result.
Comment 3 Elson Yuen CLA 2010-08-25 14:49:00 EDT 
Created attachment 177458 [details]
v1.0

This patch addresses the problems that I have mentioned on the previous comment.
Comment 4 Angel Vera CLA 2010-08-31 22:37:03 EDT 
* Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. 
Although not a stop-ship. Performance fixes are always better to be included and considered important for the overall performance of the product.

* Is there a work-around? If so, why do you believe the work-around is insufficient? 
No workaround available.

* How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? 
All JUnits run and some manual test.

* Give a brief technical overview. Who has reviewed this fix? 
I(Angel) reviewed the fix. The changes look good.

* What is the risk associated with this fix? 
Medium, since we are now making use of proper caching there could be a potential problem if the cache is not maintain properly. The proper testing of this fix minimizes the risk.
Comment 5 Angel Vera CLA 2010-08-31 22:38:17 EDT 
(In reply to comment #3)

The patch is missing the copyright updates, so I will include that when I commit the code.
Comment 6 Angel Vera CLA 2010-08-31 22:47:22 EDT 
Thanks for the fast PMC approval. 

Changes committed and released to 32M
Comment 7 Angel Vera CLA 2010-08-31 22:50:44 EDT 
Changes committed and released to HEAD (3.3)
Comment 8 Eclipse Genie CLA 2017-10-11 16:34:38 EDT 
New Gerrit change created: https://git.eclipse.org/r/108988