Skip to content

Conversation

danbev
Copy link
Member

@danbev danbev commented Aug 23, 2025

This commit removes the userdata parameter from the WebGPU request adapter callback in ggml-webgpu.cpp. Instead, the lambda function captures the webgpu_context directly.

The motivation for this change is to simplify the code and improve readability.

This commit removes the `userdata` parameter from the WebGPU request
adapter callback in `ggml-webgpu.cpp`. Instead, the lambda function
captures the `webgpu_context` directly.

The motivation for this change is to simplify the code and improve
readability.
@github-actions github-actions bot added the ggml changes relating to the ggml tensor library for machine learning label Aug 23, 2025
Copy link
Collaborator

@ngxson ngxson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer keeping void * userdata because:

  • It's possible to make the callback a static function
  • More crucially, it maintains the correct callback function signature defined here

@danbev
Copy link
Member Author

danbev commented Aug 26, 2025

I prefer keeping void * userdata because:

  • It's possible to make the callback a static function
  • More crucially, it maintains the correct callback function signature defined here

I was just thinking that since the implementation is using webgpu/webgpu_cpp.h it might be a nice change in favor of somewhat simpler/more readable code (at least in my opinion).

@ngxson
Copy link
Collaborator

ngxson commented Aug 26, 2025

Hmm ok I didn't notice that webgpu_cpp supports both version of callback (w/ and w/o userdata)

Probably it's also cleaner to move the callback to the same line with function call, like in this example: https://eliemichel.github.io/LearnWebGPU/getting-started/cpp-idioms.html#capturing-closures

This commit removes the callback lambda variable and inlines it directly
into the RequestAdapter call.
Copy link
Collaborator

@ngxson ngxson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like a good change, is there anything blocking this from being merged?

@danbev
Copy link
Member Author

danbev commented Sep 7, 2025

seems like a good change, is there anything blocking this from being merged?

Nothing from my end 👍

@ggerganov ggerganov merged commit 3b15924 into ggml-org:master Sep 7, 2025
88 of 89 checks passed
@danbev danbev deleted the webgpu-request-adapter-callback-userdata branch September 9, 2025 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants