PoC: Implement basic per-context module data #1723
Conversation
|
... and calling |
gabrielschulhof
force-pushed
the
context-module-list
branch
from
89a77b1 to
ce7a226
Compare
| */ | ||
|
|
||
| /* This value is context-independent */ | ||
| static int module_index = -1; |
There was a problem hiding this comment.
does "context-independent" means all context share the same index? if so, Why?
IMO, the index should be in jerry_context, and each context can have different modules.
There was a problem hiding this comment.
Because the index counts the modules that were registered in the process not the instances of the modules that were started in a context.
I originally had the index in the context but I realized that jerry_register_module() will only be called once for each module, no matter how many contexts are later created.
There was a problem hiding this comment.
So, like, slot 0 belongs to canvas and slot 1 belongs to sqlite, etc. When you start a new context, then slot 0 in that context will belong to the instance of canvas loaded into that context, and slot 1 in that context will belong to sqlite loaded into that context, and the same for every subsequent context.
There was a problem hiding this comment.
I guess I should add a test with two trivial modules to test the data handholding.
There was a problem hiding this comment.
Yeah, in the proposal that arose from #1717, module indices are global (and assigned once per boot-up).
That means that if multiple contexts use different sets of modules, there will be unused slots in the module_data array. I think this is probably acceptable.
The alternative is to add another per-context module registration step and another level of indirection. That would also add extra memory, so I'm not convinced that at the bottom line this would be better compared to having a couple unused slots.
There was a problem hiding this comment.
I see, it makes sense.
jerry-core/jerryscript.h
Outdated
| */ | ||
| int jerry_register_module(void); | ||
| void *jerry_get_module_data(int index); | ||
| void jerry_set_module_data(int index, void *data, void (*deleter)(void *)); |
There was a problem hiding this comment.
How do you know the module index in a module?
There was a problem hiding this comment.
global static, assigned only once when the constructor gets called (implemented ideally via __attribute__((constructor)) but an exported function that is called from main() will work too).
|
Man |
gabrielschulhof
force-pushed
the
context-module-list
branch
from
d90b1f3 to
9746d90
Compare
|
OK, I dunno how to fix the rest of the linting errors 🤷 |
jerry-core/jcontext/jcontext.h
Outdated
| @@ -103,6 +113,7 @@ typedef struct | |||
| uint8_t valgrind_freya_mempool_request; /**< Tells whether a pool manager | |||
| * allocator request is in progress */ | |||
| #endif /* JERRY_VALGRIND_FREYA */ | |||
| jerry_module_data_t module_data[JERRY_CONTEXT_MODULE_COUNT]; | |||
There was a problem hiding this comment.
My guess is we'll want to have a "feature-flag" (#define / #ifdef) around all of this so that people still have the option to go for the most bare bones config possible.
| */ | ||
|
|
||
| /* This value is context-independent */ | ||
| static int module_index = -1; |
There was a problem hiding this comment.
Yeah, in the proposal that arose from #1717, module indices are global (and assigned once per boot-up).
That means that if multiple contexts use different sets of modules, there will be unused slots in the module_data array. I think this is probably acceptable.
The alternative is to add another per-context module registration step and another level of indirection. That would also add extra memory, so I'm not convinced that at the bottom line this would be better compared to having a couple unused slots.
|
I think native module management should not be part of JerryScript, which is an ECMAScript engine. This should be done in the utility library, where Standard Modules could use a nice generic interface. This could be one implementation of the interface. What do you think? |
|
@gabrielschulhof please check the discussion in #1717 |
|
@zherczeg so ... move to a new |
|
In #1716 @martijnthe proposed 3 places for such utilities. Perhaps you could also vote. |
Given an expected number of modules (JERRY_CONTEXT_MODULE_COUNT) which can be defined at compile time, reserve two pointers in each context for module-specific data: one void * for the data, and a function pointer for the deleter to be called when the context is discarded. The modules receive an index into the data array when they register. They can later use the index to store data in the context. Re jerryscript-project#1717 JerryScript-DCO-1.0-Signed-off-by: Gabriel Schulhof [email protected]
gabrielschulhof
force-pushed
the
context-module-list
branch
2 times, most recently
from
e7126c9 to
ebf07cf
Compare
|
Is this PR still valid? |
Given an expected number of modules (JERRY_CONTEXT_MODULE_COUNT) which
can be defined at compile time, reserve two pointers in each context for
module-specific data: one void * for the data, and a function pointer
for the deleter to be called when the context is discarded.
The modules receive an index into the data array when they register.
They can later use the index to store data in the context.
Re #1717
@martijnthe is this like what you had in mind?
@zherczeg note that when JERRY_CONTEXT_MODULE_COUNT is defined as 1 then this is exactly the single pointer case.