-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
[Discussion] FFI - export os.sizeof and os.alignof mappings
#1759
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
Changes from 4 commits
db3c177
3660bd6
24ac6f0
3432054
f920871
76dcf72
35fbb99
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a specific reason why this is in a new file rather than
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No particularly good reason, but I was planning on using
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay! I don't have much experience with the C++ anyways, but perhaps someone else may have an opinion on it. |
||
| #include "env.h" | ||
| #include "env-inl.h" | ||
| #include "node.h" | ||
| #include "node_internals.h" | ||
| #include "v8-profiler.h" | ||
| #include "v8.h" | ||
|
|
||
| namespace node { | ||
| namespace _alignof { | ||
|
|
||
| using v8::Context; | ||
| using v8::Handle; | ||
| using v8::Object; | ||
| using v8::Persistent; | ||
| using v8::Uint32; | ||
| using v8::Value; | ||
|
|
||
|
|
||
| void Initialize(Handle<Object> exports, | ||
| Handle<Value> unused, | ||
| Handle<Context> context) { | ||
| Environment* env = Environment::GetCurrent(context); | ||
|
|
||
| #define SET_ALIGNOF(name, type) \ | ||
| struct s_##name { type a; }; \ | ||
| exports->Set(FIXED_ONE_BYTE_STRING(env->isolate(), #name ), \ | ||
| Uint32::NewFromUnsigned(env->isolate(), static_cast<uint32_t>(__alignof__(struct s_##name)))); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why don't you use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I forget we're in C++ land sometimes :) Does that mean I can ditch the struct thing?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also, long line.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, no need for the struct. |
||
|
|
||
| // fixed alignments | ||
| SET_ALIGNOF(int8, int8_t); | ||
| SET_ALIGNOF(uint8, uint8_t); | ||
| SET_ALIGNOF(int16, int16_t); | ||
| SET_ALIGNOF(uint16, uint16_t); | ||
| SET_ALIGNOF(int32, int32_t); | ||
| SET_ALIGNOF(uint32, uint32_t); | ||
| SET_ALIGNOF(int64, int64_t); | ||
| SET_ALIGNOF(uint64, uint64_t); | ||
| SET_ALIGNOF(float, float); | ||
| SET_ALIGNOF(double, double); | ||
| SET_ALIGNOF(longdouble, long double); | ||
|
|
||
| // (potentially) variable alignments | ||
| SET_ALIGNOF(bool, bool); | ||
| SET_ALIGNOF(char, char); | ||
| SET_ALIGNOF(uchar, unsigned char); | ||
| SET_ALIGNOF(short, short); | ||
| SET_ALIGNOF(ushort, unsigned short); | ||
| SET_ALIGNOF(int, int); | ||
| SET_ALIGNOF(uint, unsigned int); | ||
| SET_ALIGNOF(long, long); | ||
| SET_ALIGNOF(ulong, unsigned long); | ||
| SET_ALIGNOF(longlong, long long); | ||
| SET_ALIGNOF(ulonglong, unsigned long long); | ||
| SET_ALIGNOF(pointer, char *); | ||
| SET_ALIGNOF(size_t, size_t); | ||
|
|
||
| // alignment of a Persistent handle to a JS object | ||
| SET_ALIGNOF(Object, Persistent<Object>); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain the need for this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll remove it (copy and paste oversight). But as an explanation, in |
||
|
|
||
| #undef SET_ALIGNOF | ||
| } | ||
|
|
||
|
|
||
| } // namespace _alignof | ||
| } // namespace node | ||
|
|
||
| NODE_MODULE_CONTEXT_AWARE_BUILTIN(alignof, node::_alignof::Initialize) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
|
|
||
| #include "env.h" | ||
| #include "env-inl.h" | ||
| #include "node.h" | ||
| #include "node_internals.h" | ||
| #include "v8-profiler.h" | ||
| #include "v8.h" | ||
|
|
||
| namespace node { | ||
| namespace _sizeof { | ||
|
|
||
| using v8::Context; | ||
| using v8::Handle; | ||
| using v8::Isolate; | ||
| using v8::Object; | ||
| using v8::Persistent; | ||
| using v8::Uint32; | ||
| using v8::Value; | ||
|
|
||
|
|
||
| void Initialize(Handle<Object> exports, | ||
| Handle<Value> unused, | ||
| Handle<Context> context) { | ||
| Environment* env = Environment::GetCurrent(context); | ||
|
|
||
| // fixed sizes | ||
| #define SET_SIZEOF(name, type) \ | ||
| exports->Set(FIXED_ONE_BYTE_STRING(env->isolate(), #name ), \ | ||
| Uint32::NewFromUnsigned(env->isolate(), static_cast<uint32_t>(sizeof( type )))); | ||
|
|
||
| SET_SIZEOF(int8, int8_t); | ||
| SET_SIZEOF(uint8, uint8_t); | ||
| SET_SIZEOF(int16, int16_t); | ||
| SET_SIZEOF(uint16, uint16_t); | ||
| SET_SIZEOF(int32, int32_t); | ||
| SET_SIZEOF(uint32, uint32_t); | ||
| SET_SIZEOF(int64, int64_t); | ||
| SET_SIZEOF(uint64, uint64_t); | ||
| SET_SIZEOF(float, float); | ||
| SET_SIZEOF(double, double); | ||
| SET_SIZEOF(longdouble, long double); | ||
|
|
||
| // (potentially) variable sizes | ||
| SET_SIZEOF(bool, bool); | ||
| SET_SIZEOF(byte, unsigned char); | ||
| SET_SIZEOF(char, char); | ||
| SET_SIZEOF(uchar, unsigned char); | ||
| SET_SIZEOF(short, short); | ||
| SET_SIZEOF(ushort, unsigned short); | ||
| SET_SIZEOF(int, int); | ||
| SET_SIZEOF(uint, unsigned int); | ||
| SET_SIZEOF(long, long); | ||
| SET_SIZEOF(ulong, unsigned long); | ||
| SET_SIZEOF(longlong, long long); | ||
| SET_SIZEOF(ulonglong, unsigned long long); | ||
| SET_SIZEOF(pointer, char *); | ||
| SET_SIZEOF(size_t, size_t); | ||
|
|
||
| // size of a Persistent handle to a JS object | ||
| SET_SIZEOF(Object, Persistent<Object>); | ||
|
|
||
| #undef SET_SIZEOF | ||
| } | ||
|
|
||
|
|
||
| } // namespace _sizeof | ||
| } // namespace node | ||
|
|
||
| NODE_MODULE_CONTEXT_AWARE_BUILTIN(sizeof, node::_sizeof::Initialize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better to cache a return result here? (Or is that already what the c++ does?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The C++ layer already caches
process.binding()results.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok cool, so long as this doesn't have to go through c++ each time it is accessed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lib/os.jsalso is only executed once (and the exports are cached), so yes, the values should basically be cached indefinitely.