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

Metal direct property access #2167

Closed
kvark opened this issue Jun 21, 2018 · 8 comments
Closed

Metal direct property access #2167

kvark opened this issue Jun 21, 2018 · 8 comments

Comments

@kvark
Copy link
Member

kvark commented Jun 21, 2018

I noticed that native ObjC apps access the properties directly, while we send messages (as setSomeProperty and someProperty). This could explain why we are spending much more time (up to 7% total) in the objC message sending.

@kvark
Copy link
Member Author

kvark commented Jun 21, 2018

cc @SSheldon @jrmuizel @grovesNL

@jrmuizel
Copy link
Contributor

jrmuizel commented Jun 21, 2018 via email

@kvark
Copy link
Member Author

kvark commented Jun 21, 2018

@jrmuizel

Objective C property accesses normally happen through objc_msgSend

That's not what I see. Molten just accesses the fields as:

MTLRenderPassColorAttachmentDescriptor* mtlColorAttDesc = mtlRPDesc.colorAttachments[caIdx];
mtlColorAttDesc.clearColor = ...

While we go through messages:

impl RenderPassColorAttachmentDescriptorRef {
    pub fn clear_color(&self) -> MTLClearColor {
        unsafe {
            msg_send![self, clearColor]
        }
    }

    pub fn set_clear_color(&self, clear_color: MTLClearColor) {
        unsafe {
            msg_send![self, setClearColor:clear_color]
        }
    }
}

If you put up instructions for reproducing the profile some place I can take a look.

would it help if I just send you the profile I got? I suppose you may not see the debug symbols then...

Anyhow, here are the (rough unverified) instructions:

# install Steam and Dota
# go to Steam - >Dota2 -> Properties -> DLC and select "Dota 2 - Vulkan Support"
git clone https://github.com/gfx-rs/portability
ln -s "/Users/<your_name>/Library/Application Support/Steam/steamapps/common/dota 2 beta/game" dota2
cd portability
make version-release
# install required dependencies, e.g. XCode, if the build fails
ln -s <this_folder>/target/release/libportability.dylib target/release/libMoltenVK.dylib
make dota-release
# check if Settings -> Video says it's running Vulkan. The console output shouldn't have any "mvk" stuff

Thank you in advance for looking into it!

@SSheldon
Copy link

SSheldon commented Jun 21, 2018

@kvark dot-syntax for properties in Objective-C is just syntactic sugar for method calls :) The compiler desugars foo.bar = ... to [foo setBar:...] and ... = foo.bar to ... = [foo bar].

Having trouble finding an official reference for this, but here's a corroborating stack overflow at least: https://stackoverflow.com/a/7423901

@grovesNL
Copy link
Contributor

More generally, is there any downside to avoiding the overhead of msg_send entirely by caching function pointers? SSheldon/rust-objc#64 improves the selectors but we still speed a lot of time in objc_msgSend

I was thinking we could have a lazy_imp to replace msg_send for all selectors in metal-rs. lazy_imp would cache the function pointer (similar to the selector cache in SSheldon/rust-objc#64)

@jrmuizel
Copy link
Contributor

objc_msgSend already has a method cache so caching function pointers might a huge amount faster. Caching function pointers also has the problem of not knowing when the cache is wrong because the method changed. Also, clang isn't caching function pointers and we see much see much less time in objc_msgSend with MoltenVK than with gfx, which suggests that there's a bigger problem.

@grovesNL
Copy link
Contributor

@jrmuizel agreed, there are other inefficiencies that we need to fix (we probably call objc_msgSend a lot more in general). Although I still wonder how much overhead the combined selector caching + message send adds to every property/method call.

Caching function pointers also has the problem of not knowing when the cache is wrong because the method changed

Is it typical for APIs like Metal to change methods dynamically?

@kvark
Copy link
Member Author

kvark commented Jun 28, 2018

Alright, let's discuss the possible optimization of caching the function pointers in a dedicated issue. This one is closed, thanks to explanations above.

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

No branches or pull requests

4 participants