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

Using fixed-length arrays for some output arrays? #302

Closed
kainino0x opened this issue Jun 10, 2024 · 4 comments
Closed

Using fixed-length arrays for some output arrays? #302

kainino0x opened this issue Jun 10, 2024 · 4 comments
Labels
has resolution Issue is resolved, just needs to be done

Comments

@kainino0x
Copy link
Collaborator

In Emscripten there were a few suggestions to use fixed-length arrays for some outputs to avoid needing FreeMembers. In particular AdapterInfo just has a few strings in it - we probably could get away with setting some fixed limit on their size, but description is the most unsure, since it might get wrapped repeatedly the way ANGLE does:
ANGLE (Apple, ANGLE Metal Renderer: Apple M1 Pro, Version 14.5 (Build 23F79))

Discussion was here: emscripten-core/emscripten#22031 (comment)

@Kangz
Copy link
Collaborator

Kangz commented Jun 10, 2024

Do we know what the cost of a malloc is in emscripten? Something like AdapterGetInfo happens maybe once per application run so it doesn't seem super worth optimizing by having some arbitrary length hardcoded.

@kainino0x
Copy link
Collaborator Author

I'm definitely not concerned with the performance cost for AdapterGetInfo, given it should be called basically once. It's more of an ergonomic improvement as you wouldn't need FreeMembers at all for this struct. It's feasible to do similar things for other structs as well, like changing presentModes to a fixed-size array.

@Kangz
Copy link
Collaborator

Kangz commented Jun 11, 2024

Ah, that would be a problem only for C though and only in a few places so not super important either imho.

@kainino0x
Copy link
Collaborator Author

kainino0x commented Jul 18, 2024

Jul 18 meeting:

  • e.g. char vendor[256], instead of char const * vendor + FreeMembers
  • CF: Seems like an over-optimization
  • KN: Think it would more of an ergonomic improvement than anything
  • CF: Prefer to have consistency since some places will need FreeMembers
  • Leave as is

@kainino0x kainino0x closed this as not planned Won't fix, can't repro, duplicate, stale Jul 18, 2024
@kainino0x kainino0x added has resolution Issue is resolved, just needs to be done and removed !discuss Needs discussion (at meeting or online) labels Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has resolution Issue is resolved, just needs to be done
Projects
None yet
Development

No branches or pull requests

2 participants