Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash on 32/64 bit #1

Closed
moremachinethanman opened this issue Jan 13, 2015 · 19 comments
Closed

Crash on 32/64 bit #1

moremachinethanman opened this issue Jan 13, 2015 · 19 comments
Assignees

Comments

@moremachinethanman
Copy link

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
Copy link

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

@ariccio
Copy link
Owner

ariccio commented Jan 21, 2015

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 ariccio self-assigned this Jan 21, 2015
@ariccio
Copy link
Owner

ariccio commented Jan 21, 2015

(slash sorry for the 8-day delay)

@mcoliver
Copy link

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
#1 (comment).

Michael Oliver
mcoliver@gmail.com
858.336.1438

ariccio added a commit that referenced this issue Jan 22, 2015
I've audited every single use of std::terminate( )/abort( ) (this is why
I keep the codebase small!), an ensured that at the very least, some
form of error message is displayed before any termination, no matter how
unexpected.

Also fixed my own idiotic use of `void displayWindowsMsgBoxWithMessage(
PCWSTR )` whereby I'd construct a std::wstring from a (wide) string
literal, then move it into `void displayWindowsMsgBoxWithMessage( const
std::wstring )`. What the hell was I doing??

Lastly, I disabled AVX generation, and now uses the SSE2 option for 32
bit builds, and the DEFAULT option for 64 bit builds. All 64 bit
processors have SSE2 built in, and MSVC doesn't even recognize
`/arch:SSE2` when building a 64 bit executable.

Hopefully this addresses the issue!
@ariccio
Copy link
Owner

ariccio commented Jan 22, 2015

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
Copy link

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
#1 (comment).

Michael Oliver
mcoliver@gmail.com
858.336.1438

@ariccio
Copy link
Owner

ariccio commented Jan 27, 2015

@mcoliver
Copy link

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
#1 (comment).

Michael Oliver
mcoliver@gmail.com
858.336.1438

@ariccio
Copy link
Owner

ariccio commented Jan 29, 2015

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

@mcoliver
Copy link

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
#1 (comment).

Michael Oliver
mcoliver@gmail.com
858.336.1438

@ariccio
Copy link
Owner

ariccio commented Jan 29, 2015

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
Copy link
Owner

ariccio commented Jan 29, 2015

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
Copy link
Owner

ariccio commented Jan 30, 2015

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):

  • /analyze (Visual Studio 2013's static analysis tool) is fantastic. Any developer who isn't using it, is making a mistake.
  • 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 incredibly useful, and it's detected many many subtle bugs, and generally improved code quality.

@mcoliver
Copy link

mcoliver commented Feb 6, 2015

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
#1 (comment).

Michael Oliver
mcoliver@gmail.com
858.336.1438

@ariccio
Copy link
Owner

ariccio commented Feb 7, 2015

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

I'm assuming you did so from email?

@mcoliver
Copy link

mcoliver commented Feb 7, 2015

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
#1 (comment).

Michael Oliver
mcoliver@gmail.com
858.336.1438

@ariccio
Copy link
Owner

ariccio commented Feb 7, 2015

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
Copy link
Owner

ariccio commented Feb 9, 2015

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

@ariccio
Copy link
Owner

ariccio commented Feb 10, 2015

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants