patternjavaModerate
HTTP server and multi-threading optimization
Viewed 0 times
multiandoptimizationthreadinghttpserver
Problem
I wrote an HTTP server for the management of scores for users and levels.
It can return the highest score per level. It has a simple login with session-key.
What do you think could be improved in this code? What's wrong? And particularly, are there any multi-threading issues that I didn't take into account?
```
//****
//
// Http Server
//
// Author: Bombo Gongo
//
// Class purpose: demonstration of an http server
//
//***
package org.bombo.server;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.util.concurrent.Executors;
import com.sun.net.httpserver.HttpContext;
import com.sun.net.httpserver.HttpServer;
public class Main {
private static int port = 8009;
public static void main(String[] args) throws IOException {
// Create an http server
HttpServer server = HttpServer.create(new InetSocketAddress(port), 0);
// Create a context
HttpContext context = server.createContext("/", new RequestsHandler());
// Add a filter
context.getFilters().add(new ParamsFilter());
// Set an Executor for the multi-threading
server.setExecutor(Executors.newCachedThreadPool());
// Start the server
server.start();
System.out.println("The server is started!");
}
}
//****
//
// Http Server
//
// Author: Bombo Gongo
//
// Class purpose: filter to set the request parameters
//
//***
package org.bombo.server;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.util.HashMap;
import java.util.Map;
import org.bombo.util.MiscUtils;
import com.sun.net.httpserver.*;
public class ParamsFilter extends Filter {
MiscUtils u
It can return the highest score per level. It has a simple login with session-key.
What do you think could be improved in this code? What's wrong? And particularly, are there any multi-threading issues that I didn't take into account?
```
//****
//
// Http Server
//
// Author: Bombo Gongo
//
// Class purpose: demonstration of an http server
//
//***
package org.bombo.server;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.util.concurrent.Executors;
import com.sun.net.httpserver.HttpContext;
import com.sun.net.httpserver.HttpServer;
public class Main {
private static int port = 8009;
public static void main(String[] args) throws IOException {
// Create an http server
HttpServer server = HttpServer.create(new InetSocketAddress(port), 0);
// Create a context
HttpContext context = server.createContext("/", new RequestsHandler());
// Add a filter
context.getFilters().add(new ParamsFilter());
// Set an Executor for the multi-threading
server.setExecutor(Executors.newCachedThreadPool());
// Start the server
server.start();
System.out.println("The server is started!");
}
}
//****
//
// Http Server
//
// Author: Bombo Gongo
//
// Class purpose: filter to set the request parameters
//
//***
package org.bombo.server;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.util.HashMap;
import java.util.Map;
import org.bombo.util.MiscUtils;
import com.sun.net.httpserver.*;
public class ParamsFilter extends Filter {
MiscUtils u
Solution
- Please think twice before using your own http server. The better way is using Jetty or embed Tomcat - good way to get rid of defects and to save time for developing business logic.
-
If you're going to use multithreading please look into java.util.concurrent.atomic package in order to start using AtomicInteger etc.
-
Your multithreading is not actually multithreading as you've wrapped all logic into synchronized block. This will cause all threads to be executed one by one.
-
Map
>> is not a good way of handling variables. This might be an object. Probably you can make it thread safe inside-
volatile on Map is useless. It's like putting final on it. Anyway usage of these modifiers is not recommended as you can handle all types by concurrent package.
-
This point is only my opinion but I do not like using TreeMap. As far as I see you are trying to return sorted String. But you can get values from scoreResults (
map.values()) and sort them with Collections.sort. Also you should look into StringUtils.join() method (apache.commons package) in order to receive String. This will give you more readable code.-
Always close streams in try catch with finally block and always consume response body as you can receive memory leaks in your code:
os.write(response.toString().getBytes());E.g. if the line above will cause an exception then os stream won't be closed. This will cause memory leak in your server. Please take a look in
IOUtils.closeQuitely() method (apache.commons.io) as an exampleHope this will help you
Context
StackExchange Code Review Q#24740, answer score: 10
Revisions (0)
No revisions yet.