search

ariccio / altWinDirStat

An unofficial modification of WinDirStat
Other
623 stars 41 forks source link

Crash on 32/64 bit #1

Closed moremachinethanman closed 9 years ago

moremachinethanman commented 9 years ago

Hi Alexander,

First, thanks for this fork! I tried both builds available and they both crash on startup. I have 2 local hard drives and 8 mapped drives.

The fault observed indicated the following: Problem Event Name: APPCRASH Application Name: altwindirstat_64.exe Application Version: 0.0.0.80 Application Timestamp: 54b451d2 Fault Module Name: altwindirstat_64.exe Fault Module Version: 0.0.0.80 Fault Module Timestamp: 54b451d2 Exception Code: 40000015 Exception Offset: 00000000001c45c2 OS Version: 6.1.7601.2.1.0.256.48 Locale ID: 4105 Additional Information 1: a51e Additional Information 2: a51e09ffba5ab78abe532150238352bc Additional Information 3: 3d88 Additional Information 4: 3d8891582a2dccdd64ca0d1ebfdd2fdf

I'd be happy to help debug, but don't have development tools on this workstation.

Best, Chris

mcoliver commented 9 years ago

Both binaries are crashing for me as well (Win 7 x64 SP1)

ariccio commented 9 years ago

Does your processor support AVX? Right now I compile with AVX, but am strongly considering dropping that. It's too bad that VS2013 doesn't support dynamic code path dispatch (Intel's does).

Thanks for reporting the crash!!

ariccio commented 9 years ago

(slash sorry for the 8-day delay)

mcoliver commented 9 years ago

Yes...Processor is a i7-2600. Has AVX but not AVX2.

http://ark.intel.com/products/52213/Intel-Core-i7-2600-Processor-8M-Cache-up-to-3_80-GHz

On Tue, Jan 20, 2015 at 9:27 PM, Alexander Riccio notifications@github.com wrote:

(slash sorry for the 8-day delay)

Reply to this email directly or view it on GitHub https://github.com/ariccio/altWinDirStat/issues/1#issuecomment-70786594.

Michael Oliver mcoliver@gmail.com 858.336.1438

ariccio commented 9 years ago

Alrighty, let's see if v0.9.1 fixes anything.

If it doesn't (it still crashes silently) what's the last message the debug version outputs?

mcoliver commented 9 years ago

Great. Thanks! Do you have a link for the download?

On Wed, Jan 21, 2015 at 8:43 PM, Alexander Riccio notifications@github.com wrote:

Alrighty, let's see if v0.9.1 fixes anything.

If it doesn't (it still crashes silently) what's the last message the debug version outputs?

Reply to this email directly or view it on GitHub https://github.com/ariccio/altWinDirStat/issues/1#issuecomment-70970805.

Michael Oliver mcoliver@gmail.com 858.336.1438

ariccio commented 9 years ago

Yup! https://github.com/ariccio/altWinDirStat/releases/tag/v0.9.1

mcoliver commented 9 years ago

Nice! Thanks Alexander. Now off to scan a 100TB

On Tue, Jan 27, 2015 at 2:55 PM, Alexander Riccio notifications@github.com wrote:

Yup! https://github.com/ariccio/altWinDirStat/releases/tag/v0.9.1

Reply to this email directly or view it on GitHub https://github.com/ariccio/altWinDirStat/issues/1#issuecomment-71746049.

Michael Oliver mcoliver@gmail.com 858.336.1438

ariccio commented 9 years ago

I gotta admit, I'm curious to hear how it went!

mcoliver commented 9 years ago

yeah binary loads just fine! Thanks. However there is no "pacman" style interface to show that the program is actually scanning directories. Any chance that can be implemented? Or at least a load bar so that I know the program is not hung?

Also the + signs next to the directories should be single click not double.

Have not scanned more than a few TB's yet but looking forward to running it tonight on 100+ TB so see how it works with large data sets.

On Wed, Jan 28, 2015 at 6:49 PM, Alexander Riccio notifications@github.com wrote:

I gotta admit, I'm curious to hear how it went!

Reply to this email directly or view it on GitHub https://github.com/ariccio/altWinDirStat/issues/1#issuecomment-71960106.

Michael Oliver mcoliver@gmail.com 858.336.1438

ariccio commented 9 years ago

yeah binary loads just fine! Thanks.

Awesome. I'll close this issue.

However there is no "pacman" style interface to show that the program is actually scanning directories. Any chance that can be implemented?

Yeah, sort of. But first, a bit of storytime.

The original ('vanilla') WinDirStat's architecture was 100% single-threaded, and everything was done in the overriden CWinApp::OnIdle 'loop'. Furthermore, it'd ensure responsiveness 'simulate' a separate thread by doing very small amounts work, then checking for unprocessed window messages, then doing more work, then checking messages, etc......until the root item in the tree was done.

It was spaghetti code, and figuring out what was really going on at any point in the app's lifetime took a few weeks of on-off work. The fact that const was never used, well that really didn't help.

By UI work, I mean a simple:

while(PeekMessage(UGLY_WINDOWS_PARAMS)) {
    DispatchMessge(UGLY_WINDOWS_PARAMS);
    }

There wasn't an awful lot involved in the "pacmen" drawing code, but it touched most of the codebase, and required a lot of bookeeping.

(part 1 of 2, I'm afraid my browser will crash)

ariccio commented 9 years ago

My long-term goal was to refactor the directory enumeration, and the complexity of everything was just too much to reason about, all the while making design changes.

The directory enumeration part of WinDirStat involved, on every new file found, updating the size of all parent folders, updating the number of items in all parent folders, checking if it needed to update the pacmen, and all sorts of stuff that was painful to try and parallelize. I ended up trying ~5 separate strategies, and reverting all of them, before deciding that I'd have to cut out whole features before I'd make progress.

Right now, the directory enumeration code is completely separate from the UI code; that's the way that it should be, and it'll stay that way. So no pacmen.

Or at least a load bar so that I know the program is not hung?

I'm slowly re implementing certain features, but under the hard limit of 20,000 lines of code. This one might make it.

In the v0.9.1 build, there's a simple WTL::CWaitCursor so that the app doesn't just hang - but yes, I do believe that there should be some better indication. The current situation is the result of a simple hack so that I can use modern C++ async code for file enumeration, without trying to explain that to MFC & it's threading model. God I hate MFC.

In fact, insofar as the res of the app is concerned, the enumeration step is entirely atomic, which saves incredible complexity.

Also the + signs next to the directories should be single click not double.

Good catch. I'm a keyboard person, so I'd never have noticed! I'll get on that.

This is why it's awesome when people actually file bugs/open issues :)

( I have one more comment before I close this issue )

ariccio commented 9 years ago

Have not scanned more than a few TB's yet but looking forward to running it tonight on 100+ TB so see how it works with large data sets.

Awesome! I'm really interested to hear the results! While I can stress test with millions of files, I don't have access to a diverse enough set of systems/configurations to be satisfied. It's important to see how the app behaves in unusual (w.r.t. my dev machine) circumstances.

Although I do some testing on a machine with a classic rotating-disk hard drive, my main dev machine (the one with vTune) is an SSD, so I don't really know how well the Visual C++ implementation of std::async maxes out disk usage. The SSD is fast enough that enumeration on my computer, at the very worst, takes 10 seconds - often less than 5 - most of which is synchronization overhead in std::async, not waiting on I/O.

The reason that's important is that performance gains improve quite a bit as we queue up many I/O requests - whereby the kernel can do all sorts of optimizations that we (WinDirStat) can't, i.e. Native Command Queuing, batching, etc... Boost AFIO does a fantastic job of this, and if it were not for Boost, I'd use it.

A few side notes (not specifically for you):

mcoliver commented 9 years ago

Man this thing was fast! Still seems to max out at 3GB though even with the 64 bit edition. See below screenshot:

[image: Inline image 1]

On Thu, Jan 29, 2015 at 6:18 PM, Alexander Riccio notifications@github.com wrote:

Have not scanned more than a few TB's yet but looking forward to running it tonight on 100+ TB so see how it works with large data sets.

Awesome! I'm really interested to hear the results! While I can stress test with millions of files, I don't have access to a diverse enough set of systems/configurations to be satisfied. It's important to see how the app behaves in unusual (w.r.t. my dev machine) circumstances.

Although I do some testing on a machine with a classic rotating-disk hard drive, my main dev machine (the one with vTune) is an SSD, so I don't really know how well the Visual C++ implementation of std::async maxes out disk usage. The SSD is fast enough that enumeration on my computer, at the very worst, takes 10 seconds - often less than 5 - most of which is synchronization overhead in std::async, not waiting on I/O.

The reason that's important is that performance gains improve quite a bit as we queue up many I/O requests - whereby the kernel can do all sorts of optimizations that we (WinDirStat) can't, i.e. Native Command Queuing http://en.wikipedia.org/wiki/Native_Command_Queuing, batching, etc... Boost AFIO https://github.com/BoostGSoC13/boost.afio does a fantastic job of this, and if it were not for Boost, I'd use it.

A few side notes (not specifically for you):

  • /analyze (Visual Studio 2013's static analysis tool) is fantastic. Any developer who isn't using it, is making a mistake https://www.youtube.com/watch?v=4zgYG-_ha28#t=3485.
  • The original WDS was written in C+. That's not a typo. It was using virtual, (multiple) inheritance, MFC exceptions (which are like pseudo-exceptions), a sprinkling of almost no const, and I think twice used anonymous namespaces.
    • A tremendous part of my initial work was simply modernizing it so that I could work with the codebase, and not break everything in the process.
    • There were at least 150 implicit type conversions (they generated warnings), an obscene number of C-style casts, and a plethora of warnings.
  • All containers were MFC containers: no stdlib containers, therefore no move semantics, no small-string optimizations, no standard library algorithms, minimal type-safety, inconsistent APIs, code bloat, etc...
    • CArray, for example works with INT_PTRs, but CListCtrl uses int. Even worse, some use -1 as an error code, some use INT_MAX as an error code, so when converting, we have to check whether we're clobbering an error. This kills composability.
    • MFC exceptions are another beast entirely, there's no way to disable them, some MFC functions throw them, others don't, and because they are not compatible with std::exception, they duplicate much functionality, etc...
    • Unit testing in MFC is harder than just rigorous manual testing.
  • Lastly, I've found SAL https://msdn.microsoft.com/en-us/library/ms235402.aspx incredibly useful, and it's detected many many subtle bugs, and generally improved code quality.

Reply to this email directly or view it on GitHub https://github.com/ariccio/altWinDirStat/issues/1#issuecomment-72143073.

Michael Oliver mcoliver@gmail.com 858.336.1438

ariccio commented 9 years ago

Eh, you might need to logon to GitHub, I can't see an image.

I'm assuming you did so from email?

mcoliver commented 9 years ago

yeah from email. Here is a link: http://i.imgur.com/S1rNTeu.jpg

On Fri, Feb 6, 2015 at 5:01 PM, Alexander Riccio notifications@github.com wrote:

Eh, you might need to logon to GitHub, I can't see an image.

I'm assuming you did so from email?

Reply to this email directly or view it on GitHub https://github.com/ariccio/altWinDirStat/issues/1#issuecomment-73341054.

Michael Oliver mcoliver@gmail.com 858.336.1438

ariccio commented 9 years ago

Well, I'm sure glad that I made sure to say it's "for GetDateFormat". That made it so much easier to spot what's going on.

Of course, the windows error message "any of the parameters are invalid" is not that helpful 😉. I'll open an issue to track this, and probably be able to push out a build that displays some more debugging information on the parameters passed to GetDateFormat, by the end of the day.

ariccio commented 9 years ago

Sorry I haven't pushed out a binary yet, I'll do so tomorrow. I think I fixed it ( 85e3ac682ce2f5d86b09515bd9ef95f7bba68501 ), but I did some general cleanup too.

ariccio commented 9 years ago

Alrighty, pushed out that release. If you still have that issue, could you post in the issue for #3 ? It's better for keeping track of such things :)