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

Namespace dependencies need some cleaning up #700

Closed
kennykerr opened this issue Oct 15, 2021 · 19 comments
Closed

Namespace dependencies need some cleaning up #700

kennykerr opened this issue Oct 15, 2021 · 19 comments

Comments

@kennykerr
Copy link
Contributor

kennykerr commented Oct 15, 2021

https://gist.github.com/kennykerr/cd45cf6473498707a469abec8dd9e71e

This is from a Rust toml file showing feature dependencies. They basically map to metadata namespaces. You can see that the Win32 namespaces need some work. Note that this only shows certain dependencies (there are many others that are omitted for this list).

Reducing these dependencies would have a huge impact on build time for consuming projects.

@kennykerr
Copy link
Contributor Author

kennykerr commented Oct 15, 2021

The WinRT namespaces are relatively clean by comparison. To give you a sense for how impactful this is, here are two examples. The first example uses a simple WinRT API while the second a simple Win32 API. The WinRT API actually requires hundreds of lines of code as it involves activation factories, required interfaces, and much more. The Win32 API just requires a single function declaration.

The WinRT project creates a Windows.Foundation.Uri object. Here you can see it builds in under 5 seconds:

C:\git\windows-api-rs\examples\winrt_uri>cargo build
   Compiling const-sha1 v0.2.0
   Compiling windows v0.21.1
   Compiling windows-api v0.21.1 (C:\git\windows-api-rs)
   Compiling winrt_uri v0.0.0 (C:\git\windows-api-rs\examples\winrt_uri)
    Finished dev [unoptimized + debuginfo] target(s) in 4.39s

The Win32 project call the Windows.Win32.System.SystemInformation.GetTickCount function. Here you can see it takes two minutes to build:

C:\git\windows-api-rs\examples\win32_tick_count>cargo check
    Checking const-sha1 v0.2.0
    Checking windows v0.21.1
    Checking windows-api v0.21.1 (C:\git\windows-api-rs)
    Checking win32_tick_count v0.0.0 (C:\git\windows-api-rs\examples\win32_tick_count)
    Finished dev [unoptimized + debuginfo] target(s) in 1m 59s

This is because the GetTickCount function resides in a namespace that has direct and indirect dependencies on many other large namespaces including Direct3D9, OpenGL, and much more.

This is probably one of the more egregious examples, but many other namespaces have similarly unnecessary dependencies that impact build time.

@sotteson1
Copy link
Contributor

sotteson1 commented Oct 15, 2021

When you pull in an API from a namespace, do you generate code for the entire namespace and any dependencies? CsWin32 only pulls in the API and any types it depends on, but maybe you can't do that in Rust. I just built in an app using CsWin32 that calls GetTickCount and it built pretty much instantly.

@kennykerr
Copy link
Contributor Author

I provide a few different options to developers include generating just the function itself. While this is very fast, it includes various tradeoffs that are blocking broad adoption by the Rust ecosystem.

Having pre-generated bindings - much like C#/WinRT (not C#/Win32) - ends up being the only viable option for many projects. In the pre-generated world, you end up compiling the entire namespace.

@riverar
Copy link
Collaborator

riverar commented Oct 15, 2021

When you pull in an API from a namespace, do you generate code for the entire namespace and any dependencies? CsWin32 only pulls in the API and any types it depends on, but maybe you can't do that in Rust. I just built in an app using CsWin32 that calls GetTickCount and it built pretty much instantly.

This is how windows-rs works today for on-demand scenarios, but for pre-generated-entire-world scenario these spurious dependencies really hurt.

Another example, it appears Windows.Win32.Media.Multimedia has ties to Windows.Win32.Devices only because of this single struct. (I can file a separate bug if you wish, seems JOYREGHWVALUES is just in the wrong spot.)

// Windows.Win32.Media.Multimedia.JOYREGHWCONFIG
using System.Runtime.InteropServices;
using Windows.Win32.Devices.HumanInterfaceDevice;
using Windows.Win32.Media.Multimedia;

[StructLayout(LayoutKind.Sequential, Pack = 1)]
public struct JOYREGHWCONFIG
{
	public Windows.Win32.Media.Multimedia.JOYREGHWSETTINGS hws;
	public uint dwUsageSettings;
	public Windows.Win32.Devices.HumanInterfaceDevice.JOYREGHWVALUES hwv;
	public uint dwType;
	public uint dwReserved;
}

@sotteson1
Copy link
Contributor

I thought pre-generated bindings pre-generate all namespaces, like C#/WinRT does. I could imagine as big as the .winmd has gotten that that would be pretty slow. How does the single namespace scenario work with pre-generated bindings? Do developers choose a namespace they want to use, or is your projection choosing which ones to pre-generate? What I'm trying to figure out is how to think about fixing the problem you're running into. If you could give me some targeted namespaces you would like to see be cleaner, that would help in figuring out how to make some progress.

@kennykerr
Copy link
Contributor Author

kennykerr commented Oct 15, 2021

In Rust, developers are able to choose between:

  1. the build macro (like these samples)
  2. the generate macro (like this PR and the windows crate itself)
  3. the windows-api crate

The first two generate whatever individual APIs the developer chooses down to an individual function. The third option is a package of pre-generated bindings for the entire Windows API - ever last namespace. However each namespace is gated (much like you would use #ifdef or #include in C++) so you only compile the namespaces you actually request in your build.

So when I use option 3 and request the Windows.Foundation namespace (to use Uri) it builds very fast because that namespace has very few dependencies. But when I request the Windows.Win32.System.Com namespace (to use CreateUri) it takes forever because that namespace depends on a pile of other namespaces. Here's just a sampling:

Win32_System_Com = ["Win32_Foundation", "Win32_Graphics_Gdi", "Win32_Media_Audio_CoreAudio", "Win32_Security", "Win32_Storage_StructuredStorage", "Win32_System", "Win32_System_LibraryLoader", "Win32_System_OleAutomation", "Win32_System_SystemServices", "Win32_UI_Controls", "Win32_UI_WindowsAndMessaging"]

And those namespaces further depend on other namespaces and so on. Clearly, Com should not depend on Gdi, or CoreAudio, etc.

This is analogous to using #include "Windows.Win32.System.Com.h" on a hypothetical namespace header in C++.

@sotteson1
Copy link
Contributor

sotteson1 commented Oct 15, 2021

Windows.Win32.System.Com scrapes ole.h, which includes:

https://docs.microsoft.com/en-us/windows/win32/api/ole/nf-ole-oledraw

OLESTATUS OleDraw(
  LPOLEOBJECT ,
  HDC         ,
  const RECT  *,
  const RECT  *,
  HDC         
);

HDC comes from the Gdi namespace. To break this dependency, maybe OleDraw could go into a sub-namespace of Com like Com.UI. Or would you propose a different solution? Windows headers are notoriously enmeshed in each other, so untangling these dependencies is going to be a ton of work.

@KalleOlaviNiemitalo
Copy link

each namespace is gated (much like you would use #ifdef or #include in C++) so you only compile the namespaces you actually request in your build.

Can you gate things other than namespaces? If not, is this a limitation of Rust or just a matter of letting the developers understand which gate they need to open to get the function they want?

The Windows SDK documentation lists the header files needed for functions and types. I presume the dependencies between the header files have been reviewed. Perhaps you could gate by them.

API sets might have been another alternative but e.g. GetTickCount doesn't seem to be part of any API set.

@kennykerr
Copy link
Contributor Author

Can you gate things other than namespaces?

Namespace are the natural boundary for something like this. Otherwise, I don't see much point in having these synthesized namespaces for Win32 at all.

Windows.Win32.System.Com scrapes ole.h, which includes:

Shouldn't ole.h be included in the existing Windows.Win32.System.OleAutomation namespace instead? Or at least in its own namespace? I think having smaller, more targeted namespaces would help a lot.

@kennykerr
Copy link
Contributor Author

maybe OleDraw could go into a sub-namespace of Com like Com.UI

That could also work.

@kennykerr
Copy link
Contributor Author

Basically, you want to avoid cross-namespace dependencies other than:

  • Win32.Foundation - this namespace provides common types used by many APIs
  • Any parent namespace - it is common to depend on parent namespaces for definitions

The latter can help because it is very common for languages like C++ and Rust to always include the definitions from the parent namespace. This is what C++/WinRT does today. For example, the Windows.UI.Xaml.Controls.h header always includes the Windows.UI.Xaml.h header.

Obviously, we can't eliminate them all but we should be able to get closer to the ideal. Doing some basic analysis on the resulting win32.winmd might be easier than trying to analyze the raw headers. That's what I've been doing to great effect.

Hope that helps.

@kennykerr
Copy link
Contributor Author

By the way, the OleDraw function is not the problem. The Rust generator is smart enough to avoid dependencies from functions automatically.

The Com namespace is actually picking up the dependency on the Gdi namespace from the PICTDESC, QACONTAINER, and STGMEDIUM structs. Structs require definitions for all fields to define them correctly (in both C++ and Rust). Some of those probably make sense to move anyway. For example STGMEDIUM is for the clipboard API. Same with the CoreAudio dependency. That's coming from the OLEUIBUSYA/OLEUIBUSYW structs.

@kennykerr
Copy link
Contributor Author

In fact, almost all of the cross-namespace dependencies come from structs with field types from other namespaces. You can very easily write a script to print those out to get a sense for the scope of the problem. Deal with the structs and the problem largely goes away. That seems like a much more manageable task.

@sotteson1
Copy link
Contributor

If at all possible, we try to deal in units of headers, not structs. We can move individual structs from one namespace to another but it gets hard to manage. Sometimes we have no choice like with winuser.h which is piled with all kinds of crap that doesn't belong together.

Just looking at these three types, they all come from ocidl.h and are mostly used by things in ocidil.h and ole*.h. We could move ocidl.h out to another namespace like with the ole*.h headers. Most of the stuff in there looks specific to controls, but not all of it. Did I mention the Windows headers aren't layered very well? :)

@kennykerr
Copy link
Contributor Author

I guess I'm not clear on why the headers ultimately matter. Once you have the winmd you can freely move things around to organize as you see fit. That's already happening all over the place. It seems like that ship has sailed and you should just embrace it. 😋

@sotteson1
Copy link
Contributor

Who is moving things around after we get the .winmd? Our win32 metadata project isn't doing that.

@sotteson1
Copy link
Contributor

Also, I'm not understanding why you only want structs moved out. For the structs you listed, a bunch of interfaces and functions use those structs in the com namespace. Why is it structs cause the Rust projection problems and not the functions and interfaces that use them? It's beginning to sound like a Rust projection-specific problem you want me to solve.

@kennykerr
Copy link
Contributor Author

kennykerr commented Oct 16, 2021

Who is moving things around after we get the .winmd? Our win32 metadata project isn't doing that.

No, I mean the win32 metadata already makes decisions about how different APIs get mapped to different namespaces. Right now that's predominantly based on original header grouping but that need not be that way. The original headers are not preserved.

It's beginning to sound like a Rust projection-specific problem you want me to solve.

No, both C++ and Rust behave the same way. Structs must be fully defined because their size must be known at compile time. Functions are typically different because parameters, of reference/pointer types, don't need to be known ahead of time. So that can be deferred.

I'm not suggesting you move all or even any specific structs or headers around at all. I'm just suggesting you consider where there are cross-namespace dependencies that are undesirable that we clean those up as they negatively affect build time for languages like C++ and Rust.

It's your call - I'm just trying to give you as much information as I have and I'm certainly not suggesting we split structs from their associated functions.

@kennykerr
Copy link
Contributor Author

I think I'll just close this issue and open new issues for any specific dependency issues I come across like #701.

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

4 participants