Skip to content

Conversation

jekh
Copy link
Collaborator

@jekh jekh commented Nov 4, 2014

In my use of the proxy, I'm serving a fairly large number of requests simultaneously (several thousand, via Selenium). I started to notice some overhead from synchronization, so I decided to undertake a cleanup of synchronization-related code. This is a large changeset, so I've been testing extensively and have not yet seen any problems -- but I would appreciate feedback on other high-volume scenarios to test.

Summary of changes:

  • Synchronization updates:
    • Moved to java.util.concurrent collections if it allowed removing synchronized blocks. Almost all synchronization blocks are now gone, and there should be no synchronization when serving requests.
    • Refactored WhitelistEntry into immutable Whitelist to avoid synchronizing on the WhitelistEntry
    • Made fields in BrowserMobHttpClient volatile, since they can be set asynchronously through web service calls.
    • Made immutable fields in BrowserMobHttpClient final
  • Made UA Detector initialize-on-demand, to avoid resource use and log pollution when not using it. Updated pom latest version of UA Detector.
  • Fixed minor defect where calling stop() would not prevent proxy server from handling new requests

jekh and others added 18 commits October 24, 2014 18:04
…ctive requests. Preferring concurrent collections in java.util.concurrent when possible.
…threads may update them through the REST interface. Made some immutable fields final where possible.
…e traffic and log messages when not writing to a HAR
Conflicts:
	src/main/java/net/lightbody/bmp/proxy/ProxyManager.java
@lightbody
Copy link
Owner

Pretty cool -- I'd love to hear more about your usage that pushes so much through it!

I was ready to merge this, but sadly it doesn't auto merge. Any change you can update it (I must have pulled in another conflicting PR) and then I'll go ahead and merge it. Thanks!

Delete unused proxies after expiration time passes
Merge branch 'synchronization-cleanup'

Conflicts:
	src/main/java/net/lightbody/bmp/proxy/BlacklistEntry.java
	src/main/java/net/lightbody/bmp/proxy/ProxyManager.java
	src/main/java/net/lightbody/bmp/proxy/http/BrowserMobHttpClient.java
jekh referenced this pull request in nite23/browsermob-proxy Nov 10, 2014
@jekh
Copy link
Collaborator Author

jekh commented Nov 10, 2014

Just updated, should be good to merge now. We're using the proxy to blacklist and whitelist requests for a Selenium grid with several dozen browser sessions running simultanously, and each page makes anywhere from a couple to 50 (!) HTTP requests, resulting in a very large number of concurrent requests coming into the proxy.

Overall the proxy performs admirably. The biggest bottleneck I've found so far is threads -- each request to the Jetty server requires its own thread, so we had to reduce stack size significantly in order to avoid OutOfMemory issues caused by the threads' stacks. That's one reason I'm looking into the Jetty code, to see if we can somehow take advantage of Java's NIO to reduce the number of threads we spawn, since most of our threads are just parked waiting for the upstream server to respond. But as you pointed out, enhancing the embedded Jetty in any way is a nearly insurmountable challenge.

lightbody added a commit that referenced this pull request Nov 11, 2014
@lightbody lightbody merged commit 7903ec4 into lightbody:master Nov 11, 2014
@jekh jekh deleted the synchronization-cleanup branch November 12, 2014 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants