It does bulk reads from the underlying input stream, and is thus faster in cases where reading from that stream is costly. In your case, it’s faster to read chunks of bytes at a time from a file as opposed to a single byte at a time, as there is less disk IO.
Seems the same to me. Do I need to use something other than read()?
Are you really going to keep posting about how unlikely you think the solution is?
What do you mean?
Hmm, am I doing the buffered read wrong? cause it’s no faster than a byte by byte read for me even though I’m reading 1000 bytes at a time.
this is my code:
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.*;
import java.util.Vector;
import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.InputStream;
public class HexEditor extends JFrame{
JScrollPane hexScroll;
JScrollPane byteScroll;
JPanel panel;
JTextArea hexArea;
JTextArea byteArea;
JFileChooser chooser;
FileInputStream fin;
BufferedInputStream bin;
JMenuBar menuBar;
JMenu file;
JMenuItem load;
JProgressBar progressBar;
public HexEditor(){
super("Cypri's java hex editor");
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
progressBar = new JProgressBar();
chooser = new JFileChooser();
load = new JMenuItem("Load");
load.addActionListener(new ActionListener(){
public void actionPerformed(ActionEvent event) {
Thread hflt = new Thread(new HexFileLoader(passMe()));
hflt.start();
}
});
file = new JMenu("File");
file.add(load);
menuBar = new JMenuBar();
menuBar.add(file);
hexArea = new JTextArea();
byteArea = new JTextArea();
hexArea.setLineWrap(true);
hexArea.setWrapStyleWord(true);
byteArea.setLineWrap(true);
hexScroll = new JScrollPane(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS, ScrollPaneConstants.HORIZONTAL_SCROLLBAR_ALWAYS);
byteScroll = new JScrollPane(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS, ScrollPaneConstants.HORIZONTAL_SCROLLBAR_ALWAYS);
panel = new JPanel();
panel.add(hexScroll);
panel.add(byteScroll);
hexScroll.setViewportView(hexArea);
byteScroll.setViewportView(byteArea);
hexScroll.setPreferredSize(new Dimension(440,480));
byteScroll.setPreferredSize(new Dimension(200,480));
getContentPane().setLayout(new BorderLayout());
getContentPane().add(BorderLayout.NORTH, menuBar);
getContentPane().add(BorderLayout.CENTER, panel);
getContentPane().add(BorderLayout.SOUTH, progressBar);
pack();
setVisible(true);
}
public static byte[] hexStringToByteArray(String s) {
int len = s.length() -1;
byte[] data = new byte[(len / 2) + 1];
for (int i = 0; i < len; i += 2) {
data[i / 2] = (byte) ((Character.digit(s.charAt(i), 16) << 4) + Character.digit(s.charAt(i+1), 16));
}
return data;
}
public static char[] hexStringToByteCharArray(String s) {
int len = s.length() -1;
byte[] data = new byte[(len / 2) + 1];
char[] chardata = new char[(len / 2) + 1];
for (int i = 0; i < len; i += 2) {
data[i / 2] = (byte) ((Character.digit(s.charAt(i), 16) << 4) + Character.digit(s.charAt(i+1), 16));
chardata[i / 2] = (char) data[i / 2];
}
return chardata;
}
public void openFile(){
chooser.showOpenDialog(null);
}
public void packMe(){
pack();
}
public void revalidateMe(){
repaint();
}
public HexEditor passMe(){
return this;
}
public static void main(String[] args){
HexEditor app = new HexEditor();
}
}
class HexFileLoader implements Runnable {
HexEditor parent;
HexFileLoader(HexEditor parent) {
this.parent = parent;
}
public void run() {
try{
parent.openFile();
parent.fin = new FileInputStream(parent.chooser.getSelectedFile());
parent.bin = new BufferedInputStream(parent.fin);
System.out.println("(int) parent.chooser.getSelectedFile().getTotalSpace(): " + (int) parent.chooser.getSelectedFile().length());
parent.progressBar.setMaximum((int) parent.chooser.getSelectedFile().length());
parent.progressBar.setValue(0);
int ch;
System.out.println("Load start.");
parent.hexArea.setText("");
parent.byteArea.setText("");
int numOfBytesRead = 0;
byte[] buf = new byte[1000];
while((ch = parent.bin.read(buf)) != -1){
for(int i = 0; i < 1000; i++){
if(Integer.toHexString(buf[i]).length() < 2){
parent.hexArea.append(" 0" + Integer.toHexString(buf[i]).toUpperCase());
}
else{
parent.hexArea.append(" " + Integer.toHexString(buf[i]).toUpperCase());
}
if (ch<0x20 || ch==0x7f) {
ch = ' ';
}
parent.byteArea.append(String.valueOf((char)buf[i]));
numOfBytesRead++;
parent.progressBar.setValue(numOfBytesRead);
}
}
System.out.println("Out of loop.");
parent.packMe();
}
catch(Exception e){
e.printStackTrace();
}
}
}
That’s because you have a bunch of bottlenecks, and you just solved one (introducing a brand new bug: always reading multiples of 1000 bytes, even if there are not that many bytes)
Yeah, I figured I had that bug, but I’m more worried about the speed right now. What are some ways I can speed up my code?
Alright, it’s kinda fast now. anyone have any other ideas I can use? See any bugs?
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.*;
import java.util.Vector;
import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.InputStream;
public class HexEditor extends JFrame{
static final String HEXES = "0123456789ABCDEF";
JScrollPane hexScroll;
JScrollPane byteScroll;
JPanel panel;
JTextArea hexArea;
JTextArea byteArea;
JFileChooser chooser;
FileInputStream fin;
BufferedInputStream bin;
JMenuBar menuBar;
JMenu file;
JMenuItem load;
JProgressBar progressBar;
public HexEditor(){
super("Cypri's java hex editor");
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
progressBar = new JProgressBar();
chooser = new JFileChooser();
load = new JMenuItem("Load");
load.addActionListener(new ActionListener(){
public void actionPerformed(ActionEvent event) {
Thread hflt = new Thread(new HexFileLoader(passMe()));
hflt.start();
}
});
file = new JMenu("File");
file.add(load);
menuBar = new JMenuBar();
menuBar.add(file);
hexArea = new JTextArea();
byteArea = new JTextArea();
hexArea.setLineWrap(true);
hexArea.setWrapStyleWord(true);
byteArea.setLineWrap(true);
hexScroll = new JScrollPane(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS, ScrollPaneConstants.HORIZONTAL_SCROLLBAR_ALWAYS);
byteScroll = new JScrollPane(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS, ScrollPaneConstants.HORIZONTAL_SCROLLBAR_ALWAYS);
panel = new JPanel();
panel.add(hexScroll);
panel.add(byteScroll);
hexScroll.setViewportView(hexArea);
byteScroll.setViewportView(byteArea);
hexScroll.setPreferredSize(new Dimension(440,480));
byteScroll.setPreferredSize(new Dimension(200,480));
getContentPane().setLayout(new BorderLayout());
getContentPane().add(BorderLayout.NORTH, menuBar);
getContentPane().add(BorderLayout.CENTER, panel);
getContentPane().add(BorderLayout.SOUTH, progressBar);
pack();
setVisible(true);
}
public void openFile(){
chooser.showOpenDialog(null);
}
public void packMe(){
pack();
}
public void revalidateMe(){
repaint();
}
public HexEditor passMe(){
return this;
}
public static String getHex( byte [] raw ) {
if ( raw == null ) {
return null;
}
final StringBuilder hex = new StringBuilder( 2 * raw.length );
for ( final byte b : raw ) {
hex.append(HEXES.charAt((b & 0xF0) >> 4))
.append(HEXES.charAt((b & 0x0F)))
.append(" ");
}
return hex.toString();
}
public static void main(String[] args){
HexEditor app = new HexEditor();
}
}
class HexFileLoader implements Runnable {
HexEditor parent;
HexFileLoader(HexEditor parent) {
this.parent = parent;
}
public void run() {
try{
parent.openFile();
parent.fin = new FileInputStream(parent.chooser.getSelectedFile());
parent.bin = new BufferedInputStream(parent.fin);
System.out.println("(int) parent.chooser.getSelectedFile().getTotalSpace(): " + (int) parent.chooser.getSelectedFile().length());
parent.progressBar.setMaximum((int) parent.chooser.getSelectedFile().length());
parent.progressBar.setValue(0);
int ch;
System.out.println("Load start.");
parent.hexArea.setText("");
parent.byteArea.setText("");
int numOfBytesRead = 0;
byte[] buf;
if((int) parent.chooser.getSelectedFile().length() > 10000)
buf = new byte[10000];
else
buf = new byte[(int) parent.chooser.getSelectedFile().length()];
while((ch = parent.bin.read(buf)) != -1){
parent.hexArea.append(HexEditor.getHex(buf));
parent.byteArea.append(new String(buf));
numOfBytesRead += buf.length;
parent.progressBar.setValue(numOfBytesRead);
}
System.out.println("Out of loop.");
parent.packMe();
}
catch(Exception e){
e.printStackTrace();
}
}
}
If you’re ever trying to improve speed, the first thing to do is find where the bottleneck is. To do that, you can either use System.nanoTime() and print out time requirements in the code, or you can use a profiler like Shark.
Either way, looking at your code won’t help too much now. Instead, figure out what’s taking up all the time and cut it out.
Well, the bottleneck is right here:
parent.hexArea.append(HexEditor.getHex(buf));
parent.byteArea.append(new String(buf));
numOfBytesRead += buf.length;
parent.progressBar.setValue(numOfBytesRead);
3 UI updates for every byte…
And if that doesn’t work, run with -Xprof
Doing a bit of ‘functional’ programming:
final StringBuilder tmp1 = new StringBuilder();
final StringBuilder tmp2 = new StringBuilder();
final AtomicInteger numOfBytesRead = new AtomicInteger(); // mutable Integer (overkill due to single threaded access)
Runnable updateUI = new Runnable()
{
public void run()
{
parent.hexArea.append(tmp1.toString());
parent.byteArea.append(tmp2.toString());
parent.progressBar.setValue(numOfBytesRead.get());
tmp1.setLength(0);
tmp2.setLength(0);
}
};
while((ch = parent.bin.read()) != -1){
tmp1.append(HexEditor.getHex(ch));
tmp2.append((char)ch);
if(numOfBytesRead.incrementAndGet() % 256 == 0)
updateUI.run();
}
if(tmp2.length()>0)
updateUI.run();
Surely that’s 3 UI updates per byte buffer?
One thing that stands out immediately in the code is that there’s lots of methods being called within the Runnable that should be in the EDT. ( filechooser.getSelectedFile(), progressbar.setValue() & setMaximum(), frame.pack(), etc. )
Although the textarea.append() method is marked thread-safe, I’d still be tempted to post it in EventQueue.invokeLater(). This could speed things up quite a bit, as you’re needlessly waiting for the text area to update (and to get the EDT lock) on every buffer you process.
Yeah, assign a StringBuffer value after reading the whole file, then append the whole thing to the text area.
I’m assuming he wants to “see” the file get loaded into the hex editor chunks at a time though.
The EDT violations are a good point. Even JTextArea.append() is no longer marked threadsafe in the Java 7 Javadocs, so you should wrap them in SwingUtilities.invokeLater() as well. nsigma’s probably right that you’ll probably actually get a speed boost from it.
Another big bottleneck is that you’re using two JTextAreas with line wrap enabled. JTextArea is VERY slow wrapping long lines, and you seem to be creating a single super-long line for both text areas (well, it won’t be for byteArea whenever it encounters a ‘\n’, but still). It’ll get exponentially slower the longer the line is. Multi-MB files are not a good idea with your current setup.
If you instead fix each line as a finite size, you’ll get better performance.
You’ve also stopped replacing non-printable characters with ’ ’ in byteArea, which can make selecting the text in it slow/buggy.
Okay, I’m working on wrapping the updateUI to a SwingUtilities.invokeLater() but now when it loads a big file (around 2mb or bigger) it freezes. Here is the source:
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.*;
import java.util.Vector;
import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.InputStream;
public class HexEditor extends JFrame{
static final String HEXES = "0123456789ABCDEF";
JScrollPane hexScroll;
JScrollPane byteScroll;
JPanel panel;
JTextArea hexArea;
JTextArea byteArea;
JFileChooser chooser;
FileInputStream fin;
BufferedInputStream bin;
JMenuBar menuBar;
JMenu file;
JMenuItem load;
JProgressBar progressBar;
StringBuffer hexStr;
StringBuffer byteStr;
int numOfBytesRead;
public HexEditor(){
super("Cypri's java hex editor");
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
hexStr = new StringBuffer("");
byteStr = new StringBuffer("");
progressBar = new JProgressBar();
chooser = new JFileChooser();
load = new JMenuItem("Load");
load.addActionListener(new ActionListener(){
public void actionPerformed(ActionEvent event) {
Thread hflt = new Thread(new HexFileLoader(passMe()));
hflt.start();
}
});
file = new JMenu("File");
file.add(load);
menuBar = new JMenuBar();
menuBar.add(file);
hexArea = new JTextArea();
byteArea = new JTextArea();
hexArea.setLineWrap(true);
hexArea.setWrapStyleWord(true);
byteArea.setLineWrap(true);
hexScroll = new JScrollPane(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS, ScrollPaneConstants.HORIZONTAL_SCROLLBAR_ALWAYS);
byteScroll = new JScrollPane(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS, ScrollPaneConstants.HORIZONTAL_SCROLLBAR_ALWAYS);
panel = new JPanel();
panel.add(hexScroll);
panel.add(byteScroll);
hexScroll.setViewportView(hexArea);
byteScroll.setViewportView(byteArea);
hexScroll.setPreferredSize(new Dimension(440,480));
byteScroll.setPreferredSize(new Dimension(200,480));
getContentPane().setLayout(new BorderLayout());
getContentPane().add(BorderLayout.NORTH, menuBar);
getContentPane().add(BorderLayout.CENTER, panel);
getContentPane().add(BorderLayout.SOUTH, progressBar);
pack();
setVisible(true);
}
public void openFile(){
chooser.showOpenDialog(null);
}
public void packMe(){
pack();
}
public void revalidateMe(){
repaint();
}
public HexEditor passMe(){
return this;
}
public static String getHex( byte [] raw ) {
if ( raw == null ) {
return null;
}
final StringBuilder hex = new StringBuilder( 2 * raw.length );
for ( final byte b : raw ) {
hex.append(HEXES.charAt((b & 0xF0) >> 4))
.append(HEXES.charAt((b & 0x0F)))
.append(" ");
}
return hex.toString();
}
public static void main(String[] args){
HexEditor app = new HexEditor();
}
}
class UpdateUI implements Runnable {
HexEditor parent;
UpdateUI(HexEditor parent) {
this.parent = parent;
}
public void run() {
parent.hexArea.setText(parent.hexStr.toString());
parent.byteArea.setText(parent.byteStr.toString());
parent.progressBar.setValue(parent.numOfBytesRead);
parent.packMe();
}
}
class HexFileLoader implements Runnable {
HexEditor parent;
HexFileLoader(HexEditor parent) {
this.parent = parent;
}
public void run() {
try{
parent.openFile();
parent.fin = new FileInputStream(parent.chooser.getSelectedFile());
parent.bin = new BufferedInputStream(parent.fin);
System.out.println("(int) parent.chooser.getSelectedFile().getTotalSpace(): " + (int) parent.chooser.getSelectedFile().length());
parent.progressBar.setMaximum((int) parent.chooser.getSelectedFile().length());
parent.progressBar.setValue(0);
System.out.println("Load start.");
parent.hexStr.setLength(0);
parent.byteStr.setLength(0);
parent.numOfBytesRead = 0;
byte[] buf;
if((int) parent.chooser.getSelectedFile().length() > 10000)
buf = new byte[10000];
else
buf = new byte[(int) parent.chooser.getSelectedFile().length()];
while(parent.bin.read(buf) != -1){
parent.hexStr.append(HexEditor.getHex(buf));
parent.byteStr.append(new String(buf));
parent.numOfBytesRead += buf.length;
SwingUtilities.invokeLater(new UpdateUI(parent));
}
System.out.println("Out of loop.");
}
catch(Exception e){
e.printStackTrace();
}
}
}
I fixed some stuff like:
- taking the return of BufferedReader.read() into account
- changing to invokeAndWait() instead of invokeLater() to not spam the EvenQueue (might have caused the freeze)
- doing append([delta]) instead of setText([fullText]) on the TextAreas to prevent that drastic slow down
- removed redundant storage of the text (in the textarea and in the hexStr/byteStr vars)
- increased the read buffer
- add closing of the file
- moved stuff around a bit
Kind of works, but still waaaay slow
package cyanprime.hexedit;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.*;
import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
public class HexEditor extends JFrame
{
static final String HEXES = "0123456789ABCDEF";
JScrollPane hexScroll;
JScrollPane byteScroll;
JPanel panel;
JTextArea hexArea;
JTextArea byteArea;
JFileChooser chooser;
JMenuBar menuBar;
JMenu file;
JMenuItem load;
JProgressBar progressBar;
int numOfBytesRead;
public HexEditor()
{
super("Cypri's java hex editor");
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
progressBar = new JProgressBar();
chooser = new JFileChooser();
load = new JMenuItem("Load");
load.addActionListener(new ActionListener()
{
public void actionPerformed(ActionEvent event)
{
openFile();
}
});
file = new JMenu("File");
file.add(load);
menuBar = new JMenuBar();
menuBar.add(file);
hexArea = new JTextArea();
byteArea = new JTextArea();
hexArea.setLineWrap(true);
hexArea.setWrapStyleWord(true);
byteArea.setLineWrap(true);
hexScroll = new JScrollPane(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS, ScrollPaneConstants.HORIZONTAL_SCROLLBAR_ALWAYS);
byteScroll = new JScrollPane(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS, ScrollPaneConstants.HORIZONTAL_SCROLLBAR_ALWAYS);
panel = new JPanel();
panel.add(hexScroll);
panel.add(byteScroll);
hexScroll.setViewportView(hexArea);
byteScroll.setViewportView(byteArea);
hexScroll.setPreferredSize(new Dimension(440, 480));
byteScroll.setPreferredSize(new Dimension(200, 480));
getContentPane().setLayout(new BorderLayout());
getContentPane().add(BorderLayout.NORTH, menuBar);
getContentPane().add(BorderLayout.CENTER, panel);
getContentPane().add(BorderLayout.SOUTH, progressBar);
pack();
setVisible(true);
}
public void openFile()
{
// reset all stuff
hexArea.setText("");
byteArea.setText("");
chooser.showOpenDialog(null);
final File selectedFile = chooser.getSelectedFile();
System.out.println("(int) parent.chooser.getSelectedFile().getTotalSpace(): " + (int) selectedFile.length());
progressBar.setMaximum((int) selectedFile.length());
progressBar.setValue(0);
Thread hflt = new Thread(new HexFileLoader(selectedFile));
hflt.start();
}
public void packMe()
{
pack();
}
public void revalidateMe()
{
repaint();
}
public static String getHex(byte[] raw, int numBytes)
{
if (raw == null)
{
return null;
}
final StringBuilder hex = new StringBuilder(3 * numBytes);
for (int c = 0; c < numBytes; c++)
{
final byte b = raw[c];
hex.append(HEXES.charAt((b & 0xF0) >> 4)).append(HEXES.charAt((b & 0x0F))).append(" ");
}
return hex.toString();
}
public static void main(String[] args)
{
HexEditor app = new HexEditor();
}
class UpdateUI implements Runnable
{
final String hexChunk;
final String byteChunk;
final int numOfBytedRead;
UpdateUI(String hexChunk, String byteChunk, int numOfBytedRead)
{
this.hexChunk = hexChunk;
this.byteChunk = byteChunk;
this.numOfBytedRead = numOfBytedRead;
}
public void run()
{
HexEditor.this.hexArea.append(hexChunk);
HexEditor.this.byteArea.append(byteChunk);
HexEditor.this.progressBar.setValue(numOfBytedRead);
HexEditor.this.packMe();
}
}
class HexFileLoader implements Runnable
{
final File selectedFile;
HexFileLoader(File selectedFile)
{
this.selectedFile = selectedFile;
}
public void run()
{
BufferedInputStream bin = null;
try
{
bin = new BufferedInputStream(new FileInputStream(selectedFile));
System.out.println("Load start.");
int numOfBytesRead = 0;
byte[] buf;
buf = new byte[100000];
int numBytes = 0;
while ((numBytes = bin.read(buf)) != -1)
{
final String hexChunk = HexEditor.getHex(buf, numBytes);
final String byteChunk = new String(buf, 0, numBytes);
numOfBytesRead += numBytes;
SwingUtilities.invokeAndWait(new UpdateUI(hexChunk, byteChunk, numOfBytesRead));
}
System.out.println("Out of loop.");
}
catch (Exception e)
{
e.printStackTrace(System.err);
}
finally
{
if(bin!=null)
{
try
{
bin.close();
}
catch(IOException ex)
{
System.err.println("WARNING: error closing file! "+ex.getMessage());
}
}
}
}
}
}
Most of the changes you’ve made are good, but this one makes no sense to me. You’re effectively moving the UI update back into the processing thread by waiting for it. I don’t understand why you call it spamming the event queue? Surely the point of a queue (in most cases) is to hand over your task and get on with what you’re doing. It’s not like this cuts down on what the EventQueue is doing either, you’re just waiting for it to do it!
Also, I wonder if increasing the buffer size is actually a good idea, as you have to create a String to hold each buffer. Is lots of small objects or fewer large objects more efficient?
Actually, this is required if you’d be loading massive amounts of data: preventing one thread from producing data faster than other threads can consume it. It will, for example, potentially lead to OOME, in the general case. In this case, however, it’s not really required, although it’s a good thing to do.
Yes, I understand that, though I’d question whether to call that the “general case”. In cases of high memory usage I’d probably be tempted to use some sort of blocking queue to throttle things down when required, but not force a block each time through the loop. Even if the loading thread got rapidly ahead of the event queue in this case, it shouldn’t use much more memory than the whole file will once loaded as the Strings are eligible for GC once they’ve been appended to the text area.
Amazing random coincidence that your sig changed from impressed to disappointed to go with this post 