search

google / UIforETW

User interface for recording and managing ETW traces
https://randomascii.wordpress.com/2015/04/14/uiforetw-windows-performance-made-easier/
Apache License 2.0
1.57k stars 201 forks source link

File "ETWProvidersGenerated.h" doesn't exist! #21

Closed ariccio closed 9 years ago

ariccio commented 9 years ago

As #included in etwprof.cpp, something is broken!

randomascii commented 9 years ago

As the header-file name suggests this is a generated file. The custom build step for etwproviders.man generates that header file and two others:

%(Filename)Generated.h;%(Filename)Generated.rc;%(Filename)GeneratedTemp.bin

I just verified that ETWProviders builds cleanly from a fresh repo.

ariccio commented 9 years ago

Sorry, I misunderstood that - I'm not familiar with generating it with etwproviders.man.

Could you elaborate on how to do that?

randomascii commented 9 years ago

If you build the ETWProvider solution then ETWProvidersGenerated.h should be created. The magic incantations are built in to the .vcxproj file. Does this not work for you?

On Sun, May 3, 2015 at 3:34 PM, Alexander Riccio notifications@github.com wrote:

Sorry, I misunderstood that - I'm not familiar with generating it with etwproviders.man.

Could you elaborate on how to do that?

— Reply to this email directly or view it on GitHub https://github.com/google/UIforETW/issues/21#issuecomment-98552153.

Bruce Dawson

ariccio commented 9 years ago

It seems to work for me (now?).

I'm still quite new to manifest generation....what's the benefit here? Specifically for UIforETW?

Also: why is ETWProviders a separate solution? Any specific reason why that's better than a separate project inside the UIforETW solution?

randomascii commented 9 years ago

What's the benefit? It is the way that it is done. Microsoft mandates it, more or less. The manifest file generates a header file (that would be tedious to generate by hand) and some binary resources (undocumented format). The manifest file is how ETW events are done. I'm not sure what alternative there is.

The original reason for keeping it separate was to minimize churn in the binaries. That is, ETWProviders.dll rarely changes, and rebuilding it frequently would lead to uploading new versions frequently. If we switch to doing 'releases' then that concern may go away, but there are some logistical problems. It would probably require deleting ETWProviders.dll from the repo, which seems annoying?

On Wed, May 13, 2015 at 5:55 PM, Alexander Riccio notifications@github.com wrote:

It seems to work for me (now?).

I'm still quite new to manifest generation....what's the benefit here? Specifically for UIforETW?

Also: why is ETWProviders a separate solution? Any specific reason why that's better than a separate project inside the UIforETW solution?

— Reply to this email directly or view it on GitHub https://github.com/google/UIforETW/issues/21#issuecomment-101868526.

Bruce Dawson

ariccio commented 9 years ago

The manifest file is how ETW events are done.

OOOhhh I see. A lightbulb in my head just turned on. It all makes sense now, I remember reading something about that.

The original reason for keeping it separate was to minimize churn in the binaries. That is, ETWProviders.dll rarely changes, and rebuilding it frequently would lead to uploading new versions frequently.

That makes sense. On the other hand, as it is, if I change any of the sources for ETWProviders, and open (create?) a Pull Request, we need to merge the changes to the .dll/.lib, thus revision controlling the binaries, which might cause all sorts of problems with Git.

If it were part of the same solution, we could .gitignore the .dll and .lib, have Visual Studio build them automatically, and not need to revision control the binary. This would, however, require releases, so that users wouldn't need to compile everything themselves.

I assume that the reason why we're build a .dll and a .lib also has something to do with ETW? I'll go back and thoroughly re read everything about ETW...but that'll take a while.

I think I'll add a README.md to the ETWProviders subfolder to explain some of this.

What's the deal with C++ Exceptions in ETWProviders? I assume, because we're building a DLL that we should never throw? Maybe we should just disable them?

We're not #includeing anything from the STL at the moment, so it's not an immediate issue, but something that we should probably deal with.

Complete and total sidenote: Visual Studio insists on adding a single switch to every single solution's release build, without any configuration on my part. The switch, specifically, is /Qvec-report:2, and it causes noise in Git:

qvec_report_noise

randomascii commented 9 years ago

ETW events require a resource, which must be put in to a DLL or EXE. This is most easily done by shipping a reusable DLL that contains all of the pieces packaged up. If ETWProviders was distributed as a library containing code then users would have to link with it and also compile a resource into their EXE or DLL. Messy. Also, ETWProviders.dll can be used by many different applications and the ETW provider registration code in UIforETW makes it all work magically. If somebody has hard-core ETW needs then they can roll their own, but having a self-contained DLL definitely makes it easier.

I don't think the ETWProviders DLL creates many objects so the overhead for having exceptions enabled should be pretty minor. So it shouldn't matter whether they are enabled or not.

On Wed, May 13, 2015 at 6:24 PM, Alexander Riccio notifications@github.com wrote:

The manifest file is how ETW events are done.

OOOhhh I see. A lightbulb in my head just turned on. It all makes sense now, I remember reading something about that.

The original reason for keeping it separate was to minimize churn in the binaries. That is, ETWProviders.dll rarely changes, and rebuilding it frequently would lead to uploading new versions frequently.

That makes sense. On the other hand, as it is, if I change any of the sources for ETWProviders, and open (create?) a Pull Request, we need to merge the changes to the .dll/.lib, thus revision controlling the binaries, which might cause all sorts of problems with Git.

If it were part of the same solution, we could .gitignore the .dll and .lib, have Visual Studio build them automatically, and not need to revision control the binary. This would, however, require releases, so that users wouldn't need to compile everything themselves.

I assume that the reason why we're build a .dll and a .lib also has something to do with ETW? I'll go back and thoroughly re read everything about ETW...but that'll take a while.

I think I'll add a README.md to the ETWProviders subfolder to explain

some of this.

What's the deal with C++ Exceptions in ETWProviders? I assume, because we're building a DLL that we should never throw? Maybe we should just disable them? We're not #includeing anything from the STL at the moment, so it's not an immediate issue, but something that we should probably deal with.

Complete and total sidenote: Visual Studio insists on adding a single switch to every single solution's release build, without any configuration on my part. The switch, specifically, is /Qvec-report:2, and it causes noise in Git:

[image: qvec_report_noise] https://cloud.githubusercontent.com/assets/2142308/7624441/7cd5b372-f9b6-11e4-8aa2-b1d11a8c2b0d.PNG

— Reply to this email directly or view it on GitHub https://github.com/google/UIforETW/issues/21#issuecomment-101875105.

Bruce Dawson

ariccio commented 9 years ago

I don't think the ETWProviders DLL creates many objects so the overhead for having exceptions enabled should be pretty minor.

I'm not worried so much about the overhead, but instead am worried about ABI boundaries. If I remember correctly, emitting exceptions across an ABI boundary is a big no-no, and a DLL (as an independently compiled/loaded component) counts as an ABI boundary.

randomascii commented 9 years ago

If an exception is emitted then that's a bug. If the exception crosses the ABI boundary then that may make it crash differently, but not worse. That's why I don't care.

If ETWProviders.dll was using C++ exceptions for error handling then it would need to catch them and translate them to error codes at the ABI boundaries. But it's not. And even if it was using, say, vector, I doubt I would care about handling OOM.

On Thu, May 14, 2015 at 9:48 PM, Alexander Riccio notifications@github.com wrote:

I don't think the ETWProviders DLL creates many objects so the overhead for having exceptions enabled should be pretty minor.

I'm not worried so much about the overhead, but instead am worried about ABI boundaries. If I remember correctly, emitting exceptions across an ABI boundary is a big no-no, and a DLL (as an independently compiled/loaded component) counts as an ABI boundary.

— Reply to this email directly or view it on GitHub https://github.com/google/UIforETW/issues/21#issuecomment-102255433.

Bruce Dawson

ariccio commented 9 years ago

I wasn't even thinking about catching/translating them, but about just disabling them altogether. We'd get a crash either way, but at least we'd crash at the point of failure, instead of (god knows where else)....

I'm (just) a bit annoyed that ETWProviders is ~103kb, which is nearly 10 times bigger than WordPad! wordpadexec

...among other things, that is.

randomascii commented 9 years ago

103 KB is tiny. Yes, some things are smaller, but I don't think anybody cares much.

If disabling exceptions makes it smaller, then sure. Does it?

On Sat, May 16, 2015 at 8:26 AM, Alexander Riccio notifications@github.com wrote:

I wasn't even thinking about catching/translating them, but about just disabling them altogether. We'd get a crash either way, but at least we'd crash at the point of failure, instead of (god knows where else)....

I'm (just) a bit annoyed that ETWProviders is ~103kb, which is nearly 10 times bigger than WordPad! [image: wordpadexec] https://cloud.githubusercontent.com/assets/2142308/7665207/405d6888-fb7b-11e4-8636-8ae99eabad57.PNG

...among other things, that is http://www.phoronix.com/scan.php?px=MTY2ODc&page=news_item.

— Reply to this email directly or view it on GitHub https://github.com/google/UIforETW/issues/21#issuecomment-102583425.

Bruce Dawson