java - Thread for reading from socket stream takes more CPU usage -


in client socket, wrote thread read socket's inputstream continuously. here have used while loop read infinitely. takes more cpu; hence possible reduce cpu. please add suggestions. possible add listeners inputstream.

thread code:

public void run() { while (!shutdown) {     try {         if(socketclient != null) {             string message = socketclient.getmessage();             logger.info ("message size:" + message.length ());             if(!message.equals("emptystring")) {                 process(message);             }         }      } catch (exception exception) {         logger.info("unable read socket message" +exception);     } } 

}

socketclient.java

public class socketclient{ private volatile boolean isconnected; private int              port; private int              retrycount; private long             starttime; private string           hostname; private datainputstream  input; private dataoutputstream output; private socket           socket;  public socketclient(int port, string hostname) throws ioexception {     this.port     = port;     this.hostname = hostname;     establishconnection(); }   public void shutdown() {     try {         shutdown = true;         input.close();         output.close();         socket.close();     } catch (exception e) {         logger.debug("exception in shutdown:" + e.getmessage());     } }          public string getmessage() {     bufferedreader reader = null;      try {         stringbuilder builder = new stringbuilder();         reader = new bufferedreader(new                          inputstreamreader(tcpsocket.getinputstream()));          {             builder.append(reader.readline());         } while((reader.ready()));          if (builder.length() == 0)             return "emptystring";          return builder.tostring();     } catch (ioexception e) {             return "emptystring";     } {         try {             if(reader != null)                 reader.close();         } catch(ioexception e) {             logger.error("unable close reader");         }     } }      private void establishconnection() {         retrycount = 1;         starttime  = system.currenttimemillis();          while (!shutdown) {             try {                 if(!isconnected) {                     socket = new socket(hostname,port);                     socket.setkeepalive(true);                     input       = new datainputstream(socket.getinputstream());                     output      = new dataoutputstream(socket.getoutputstream());                     isconnected = true;                     shutdown    = true;                 }              } catch (exception exception) {                 isconnected = false;                 sleepfewseconds();                 reconnectsocket();             }         }     }      private void reconnectsocket() {         long endtime = starttime + 120000l;          if(!(system.currenttimemillis() < endtime)) {             shutdown = true;         }        }      private void sleepfewseconds() {         try {             timeunit.milliseconds.sleep(20);         } catch (interruptedexception interruptedexception) {             shutdown = true;         }     }  } 

i going critique entire class here. answer specific question appear.

public class socketclient{ private volatile boolean isconnected; 

you don't need this. socket == null well.

private int              port; private int              retrycount; private long             starttime; private string           hostname; private datainputstream  input; private dataoutputstream output; private socket           socket;  public socketclient(int port, string hostname) throws ioexception {     this.port     = port;     this.hostname = hostname;     establishconnection(); }   public void shutdown() {     try {         shutdown = true;         input.close();         output.close();         socket.close(); 

you don't need these closes, , you're doing them in wrong order anyway. output.close() sufficient , in case should first.

    } catch (exception e) {         logger.debug("exception in shutdown:" + e.getmessage());     } }  public string getmessage() {     bufferedreader reader = null; 

the bufferedreader should instance variable, not local variable. it's buffered. if make local variable lose data.

    try {         stringbuilder builder = new stringbuilder();         reader = new bufferedreader(new                          inputstreamreader(tcpsocket.getinputstream()));          {             builder.append(reader.readline());         } while((reader.ready())); 

you don't need this. if message single line, need return reader.readline(), , need caller check whether null, , if close socket, cease reading, etc. if message more 1 line, misuse of ready(): not indicator of end of message. appears comments under question shouldn't have method: connect socket input stream directly xml parser , let it reading.

        if (builder.length() == 0)             return "emptystring"; 

don't this. return "" or null. don't make new magic strings application have decode.

        return builder.tostring();     } catch (ioexception e) {             return "emptystring"; 

ditto.

    } {         try {             if(reader != null)                 reader.close(); 

you should not close reader here. closing close socket, can never message.

        } catch(ioexception e) {             logger.error("unable close reader");         }     } }  private void establishconnection() {         retrycount = 1;         starttime  = system.currenttimemillis();          while (!shutdown) {             try {                 if(!isconnected) {                     socket = new socket(hostname,port);                     socket.setkeepalive(true);                     input       = new datainputstream(socket.getinputstream());                     output      = new dataoutputstream(socket.getoutputstream());                     isconnected = true;                     shutdown    = true; 

why setting shutdown true here? nothing shutdown yet. it's brand new socket.

                }              } catch (exception exception) {                 isconnected = false;                 sleepfewseconds();                 reconnectsocket();             } 

poor practice. socket.connect(), called internally new socket(...), retries, , should distinguish between connection-failure exceptions rather adopt same strategy them all. example, 'connection timeout' have blocked minute or so: don't need another sleep; , 'connection refused' means there nothing listening, retrying pointless.

    private void reconnectsocket() {         long endtime = starttime + 120000l;          if(!(system.currenttimemillis() < endtime)) {             shutdown = true;         }        }      private void sleepfewseconds() {         try {             timeunit.milliseconds.sleep(20); 

this not 'few seconds'. 20 milliseconds, , not enough @ least 2 orders of magnite in network programming, extent there should sleep @ of course.

        } catch (interruptedexception interruptedexception) {             shutdown = true; 

shutdown appears never false. doubt you've thought through means, , doubt need @ all.

as calling code:

public void run() { while (!shutdown) {     try {         if(socketclient != null) { 

if socketclient null loop spin meaninglessly. surely method should construct socket client?

            string message = socketclient.getmessage();             logger.info ("message size:" + message.length ()); 

here failing check null , failing respond appropriately, close socket , exit loop. instead npe here.

            if(!message.equals("emptystring")) {                 process(message); 

see above. don't send special text messages. happens if peer needs send 1 day?

            }         }      } catch (exception exception) {         logger.info("unable read socket message" +exception); 

unacceptable. catch inside loop , ignores exception. result that, again, loop spin meaninglessly on exception. , methods you're calling should declared throw ioexception, , should catch here. @ present spin on nullpointerexception.


Comments

Popular posts from this blog

Command prompt result in label. Python 2.7 -

javascript - How do I use URL parameters to change link href on page? -

amazon web services - AWS Route53 Trying To Get Site To Resolve To www -