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

Add c_sizeof_object() for classes? #26015

Open
bradcray opened this issue Sep 27, 2024 · 5 comments
Open

Add c_sizeof_object() for classes? #26015

bradcray opened this issue Sep 27, 2024 · 5 comments

Comments

@bradcray
Copy link
Member

Applying a c_sizeof() to a class variable like c here:

var c = new MyClass();

returns the size of the pointer-to-object part of the class, and not the size of the class itself. This seems correct/appropriate to me, but I think we should also support a way to get the size of the object it's pointing to. Trying to come up with a user-facing way to do this was surprisingly challenging (like, I couldn't do it), and seems like a query we should make easy.

With a tip from Jade, it seems like the implementation may be as easy as:

proc c_sizeof_object(type t: class): c_size_t {
  return __primitive("sizeof_bundle", MyClassName);
}

I suppose there's some question as to whether this should be a c_ routine or just a Chapel routine given that most c_ features prefix a C symbol, and sizeof_object isn't (to my knowledge—and if it is, it probably means something different s.t. we should use a different name?).

@jabraham17
Copy link
Member

I would advocate for a name not prefixed with c_ and not in the CTypes module. To me, this feels more like a useful Chapel feature than a feature added for interop with C.

I would suggest the name sizeofType and have the symbol reside in the Types module (and to match the other symbols in Types perhaps the analogous sizeofValue, which is implemented as sizeofType(v.type))

@bradcray
Copy link
Member Author

To me, this feels more like a useful Chapel feature than a feature added for interop with C.

I'm not opposed to that, but for the sake of argument, why does it make more sense as a Chapel feature than c_sizeof() does? Or put another way, if we believe it has general utility, shouldn't we also support a Chapel-level sizeof() call?

(Note that a challenge to doing so—and a big part of why we currently only have c_sizeof(), I think?—is what it would return for a string, bytes, array, distributed array, etc. Also note that there's somewhat of an analogy here with c_addrOf() vs. c_ptrTo())

If we did think that it should be a Chapel-level routine but be class-specific, then I think we should call it sizeofClass[Object]().

@jabraham17
Copy link
Member

My thinking was that sizeofType would work on more than just a class and would tell you the size of a Chapel object in memory.

(Note that a challenge to doing so—and a big part of why we currently only have c_sizeof(), I think?—is what it would return for a string, bytes, array, distributed array, etc. Also note that there's somewhat of an analogy here with c_addrOf() vs. c_ptrTo())

Hmm I didn't think about these cases. To my mind, sizeof should return the number of bytes needed to allocate and instantiate a type. For all of these, this makes me want the "implementing record size". But of course as you point out this is the same dilemma as c_addrOf vs c_ptrTo.

@jabraham17
Copy link
Member

Note that I came across this old issue which asks for something very similar

@bradcray
Copy link
Member Author

Ooh, good catch! I'm going to add the user issues label here for that reason, but am going to keep this one open as we;; since we've had more discussion on it.

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

2 participants