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.

- 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

- changing to invokeAndWait() instead of invokeLater() to not spam the EvenQueue (might have caused the freeze)

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?

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!

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.

Hi, appreciate more people! Σ ♥ = ¾Learn how to award medals... and work your way up the social rankings!

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.

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

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.

You are right that 'in the general case' you want some kind of buffer with some size. Syncing with the thread effectively means a buffer with a size of zero: SynchronousQueue. Anyway, way offtopic.

You still haven't removed line wrapping. I removed line wrapping from your app, inserted a newline after each 10,000 'bytes' added to each text area, along with the other suggestions provided, and things were much more responsive. It took a few seconds to load a 10MB zip file, but the UI was much more responsive the entire time.

You still haven't removed line wrapping. I removed line wrapping from your app, inserted a newline after each 10,000 'bytes' added to each text area, along with the other suggestions provided, and things were much more responsive. It took a few seconds to load a 10MB zip file, but the UI was much more responsive the entire time.

You're correct. I am now working on it, and seeing extremely good results. Thank you both for your work.

To anyone who has helped in this thread, do you want to be in the credits?

There is only one queue and with invokeLater() in the loop you are inserting massive amounts of ui updates as fast as you can. So every other component using invokeLater() now has to enqueue it's work behind maybe 20 (slow) ui updates. By doing so you effectively bring swing to a halt...

There is only one queue and with invokeLater() in the loop you are inserting massive amounts of ui updates as fast as you can. So every other component using invokeLater() now has to enqueue it's work behind maybe 20 (slow) ui updates. By doing so you effectively bring swing to a halt...

In this particular case especially though, I don't think we're handing the EDT more than it can handle. I modified the code to not wrap lines, keep each line to 1024 "bytes," and to update the UI every 36 lines. On my particular machine I can load a 10MB file in 10 seconds consistently with invokeLater(), 15 seconds if I use invokeAndWait(). YMMV, of course.

In this particular case especially though, I don't think we're handing the EDT more than it can handle. I modified the code to not wrap lines, keep each line to 1024 "bytes," and to update the UI every 36 lines. On my particular machine I can load a 10MB file in 10 seconds consistently with invokeLater(), 15 seconds if I use invokeAndWait(). YMMV, of course.

That's because without wrapping and overall fewer updates (bigger buffer), the load on the queue is drastically reduced.

If you want to update the GUI more frequently (once per cell or whatever), you can always sleep the Thread and that should keep the queue from getting too big too fast. Obviously the file gets read less quickly, but you're balancing cosmetics versus functionality at that point.

There is only one queue and with invokeLater() in the loop you are inserting massive amounts of ui updates as fast as you can. So every other component using invokeLater() now has to enqueue it's work behind maybe 20 (slow) ui updates. By doing so you effectively bring swing to a halt...

OK, understand what you're getting at, but it's not like it's posting a Runnable for every character. If my late night maths is right, it's a difference of posting 15,000 characters at a time vs 150,000 characters at a time. And can the String get so big that adding it in one event actually freezes the EDT more? Personally, if we were really talking about lots of events on the EDT, I'd find a way to coalesce them, but that would probably be overkill here!

In this particular case especially though, I don't think we're handing the EDT more than it can handle. I modified the code to not wrap lines, keep each line to 1024 "bytes," and to update the UI every 36 lines. On my particular machine I can load a 10MB file in 10 seconds consistently with invokeLater(), 15 seconds if I use invokeAndWait(). YMMV, of course.

In this particular case especially though, I don't think we're handing the EDT more than it can handle. I modified the code to not wrap lines, keep each line to 1024 "bytes," and to update the UI every 36 lines. On my particular machine I can load a 10MB file in 10 seconds consistently with invokeLater(), 15 seconds if I use invokeAndWait(). YMMV, of course.

That code doesn't seem to display anything. Looks like you're missing the parent HexEditor in your calls in the HexFileLoader thread.

Aye, you're right, actually though it's just because I'm batching up the updates to the UI. It'll miss the last update (only updates on multiples of 1024*36 bytes). I missed it because I only tested on large files. Just add this right before the printing out of the runtime:

OK, understand what you're getting at, but it's not like it's posting a Runnable for every character. If my late night maths is right, it's a difference of posting 15,000 characters at a time vs 150,000 characters at a time. And can the String get so big that adding it in one event actually freezes the EDT more? Personally, if we were really talking about lots of events on the EDT, I'd find a way to coalesce them, but that would probably be overkill here!

Yeah, I tested with updating the UI after reading different amounts of bytes, and it never ran poorly (i.e. the UI was always responsive, even updating after every byte), so it just became a matter of how much you wanted to throw at it at a time. I stopped upping the buffer at 1024*36 just because it seemed "fast enough" for a simple test. Cyan can certainly tweak the buffer size to whatever he likes.

Quote

But your append() methods aren't on the EDT anyway?!

Yeah, I cheated a little, because prior to Java 7 JTextArea.append() was listed as threadsafe in its Javadoc. Unfortunately, as of Java 7, that's no longer the case. I believe they took that promise away from all the various JTextComponents' methods to modify their text. So you probably should wrap those as well.

Anyway now I am working on synchronizing the two JTextAreas. I think I have the add working correctly, but for the remove I have to override my JTextArea's Document's remove method. Anyway this is what I got so far (Modified from BoBear2681 's take on the code from: October 15, 2010, 01:38:18 pm)

(this implementation is append only, and i doubt some other things work, but removes the need for a huge array - that is a problem when you are appending and need more space, because there is a point there where you have *two* huge arrays - i had exceptions loading war and peace in my reader - only a 5 mb ascii file. Best is to know the size at the start of course.

If i were you, i'd try to use the netbeans framework. It would help you with normal infrastructure, and at the same time give you efficient text oriented components. Never tried it though, looks hard to learn, and maybe they didn't optimize large file handling.

Remember the memory problem with char[] in swing.text.* is the gapbuffer (and to a lesser extent, if you're using them, the AttributeSets), i memory profiled, but you can do the same on the netbeans profiler.I bet those crazy readers / editors that can handle gigabytes files have parsers/readers so integrated that the model you see is mapped to the file itself. When you do change something, or need to read the text they probably know to send the parser read that part of the file and apply the changes all over again to get the final result to display.

But is it really worth it, reinventing so much of the wheel (with limited features) like that, when what he has now works with files of several MB? It may be worth it in some cases I suppose.

You're right about the text package not being memory efficient though, and a better solution for a hex editor would probably be to just use a JTable with a custom TableModel backed by the byte array for the file. As of right now, a single byte is represented by 3 chars (2 hex characters and a space), but with the JTable approach each byte would just be the byte itself, and a custom renderer could be used to render if as 6A or FF, for example.

This is exactly why i think keeping Java binary compatibility forever is very, very stupid.There is lots of very, very bad code in the java std lib. Other languages update their libs so the warts are removed/reworked, but not java, oh no.

f**k it, it's the one reason i seriously considered using scala. Without projects like swinglabs it would be unbearable. Nothing is going to help you with swing.text.* - it is just not used for anything serious.

java-gaming.org is not responsible for the content posted by its members, including references to external websites,
and other references that may or may not have a relation with our primarily
gaming and game production oriented community.
inquiries and complaints can be sent via email to the info‑account of the
company managing the website of java‑gaming.org