Skip to content

Commit

Permalink
Reduce lock contention on GlobalContext.module_locks
Browse files Browse the repository at this point in the history 
Cache last module in scheduler loop to reduce lookups of modules by index
Also fix a bug in stacktraces where the lock was not held
Also remove pointer to GlobalContext from modules

This optimization, in addition to PRs atomvm#766, atomvm#770, atomvm#771 and atomvm#772, bring an
additional gain of 2-3% on Sudoku benchmark

Signed-off-by: Paul Guyot <[email protected]>
  • Loading branch information
@pguyot
pguyot committed Aug 27, 2023
1 parent d42a97e commit 081289d
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 81 deletions.
2 changes: 1 addition & 1 deletion src/libAtomVM/globalcontext.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ term globalcontext_existing_term_from_atom_string(GlobalContext *glb, AtomString
int globalcontext_insert_module(GlobalContext *global, Module *module)
{
SMP_RWLOCK_WRLOCK(global->modules_lock);
AtomString module_name_atom = module_get_atom_string_by_id(module, 1);
AtomString module_name_atom = module_get_atom_string_by_id(module, 1, global);
if (!atomshashtable_insert(global->modules_table, module_name_atom, TO_ATOMSHASHTABLE_VALUE(module))) {
SMP_RWLOCK_UNLOCK(global->modules_lock);
return -1;
Expand Down
49 changes: 24 additions & 25 deletions src/libAtomVM/module.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@
#endif
static struct LiteralEntry *module_build_literals_table(const void *literalsBuf);
static void module_add_label(Module *mod, int index, void *ptr);
static enum ModuleLoadResult module_build_imported_functions_table(Module *this_module, uint8_t *table_data);
static enum ModuleLoadResult module_build_imported_functions_table(Module *this_module, uint8_t *table_data, GlobalContext *glb);
static void parse_line_table(uint16_t **line_refs, struct ModuleFilename **filenames, uint8_t *data, size_t len);

#define IMPL_CODE_LOADER 1
#include "opcodesswitch.h"
#undef TRACE
#undef IMPL_CODE_LOADER

static enum ModuleLoadResult module_populate_atoms_table(Module *this_module, uint8_t *table_data)
static enum ModuleLoadResult module_populate_atoms_table(Module *this_module, uint8_t *table_data, GlobalContext *glb)
{
int atoms_count = READ_32_ALIGNED(table_data + 8);
const char *current_atom = (const char *) table_data + 12;
Expand All @@ -86,7 +86,7 @@ static enum ModuleLoadResult module_populate_atoms_table(Module *this_module, ui
int atom_len = *current_atom;
atom = current_atom;

int global_atom_id = globalcontext_insert_atom(this_module->global, (AtomString) atom);
int global_atom_id = globalcontext_insert_atom(glb, (AtomString) atom);
if (UNLIKELY(global_atom_id < 0)) {
fprintf(stderr, "Cannot allocate memory while loading module (line: %i).\n", __LINE__);
return MODULE_ERROR_FAILED_ALLOCATION;
Expand All @@ -100,7 +100,7 @@ static enum ModuleLoadResult module_populate_atoms_table(Module *this_module, ui
return MODULE_LOAD_OK;
}

static enum ModuleLoadResult module_build_imported_functions_table(Module *this_module, uint8_t *table_data)
static enum ModuleLoadResult module_build_imported_functions_table(Module *this_module, uint8_t *table_data, GlobalContext *glb)
{
int functions_count = READ_32_ALIGNED(table_data + 8);

Expand All @@ -113,8 +113,8 @@ static enum ModuleLoadResult module_build_imported_functions_table(Module *this_
for (int i = 0; i < functions_count; i++) {
int local_module_atom_index = READ_32_ALIGNED(table_data + i * 12 + 12);
int local_function_atom_index = READ_32_ALIGNED(table_data + i * 12 + 4 + 12);
AtomString module_atom = module_get_atom_string_by_id(this_module, local_module_atom_index);
AtomString function_atom = module_get_atom_string_by_id(this_module, local_function_atom_index);
AtomString module_atom = module_get_atom_string_by_id(this_module, local_module_atom_index, glb);
AtomString function_atom = module_get_atom_string_by_id(this_module, local_function_atom_index, glb);
uint32_t arity = READ_32_ALIGNED(table_data + i * 12 + 8 + 12);

const struct ExportedFunction *bif = bif_registry_get_handler(module_atom, function_atom, arity);
Expand Down Expand Up @@ -144,7 +144,7 @@ static enum ModuleLoadResult module_build_imported_functions_table(Module *this_
}

#ifdef ENABLE_ADVANCED_TRACE
void module_get_imported_function_module_and_name(const Module *this_module, int index, AtomString *module_atom, AtomString *function_atom)
void module_get_imported_function_module_and_name(const Module *this_module, int index, AtomString *module_atom, AtomString *function_atom, GlobalContext *glb)
{
const uint8_t *table_data = (const uint8_t *) this_module->import_table;
int functions_count = READ_32_ALIGNED(table_data + 8);
Expand All @@ -154,12 +154,12 @@ void module_get_imported_function_module_and_name(const Module *this_module, int
}
int local_module_atom_index = READ_32_ALIGNED(table_data + index * 12 + 12);
int local_function_atom_index = READ_32_ALIGNED(table_data + index * 12 + 4 + 12);
*module_atom = module_get_atom_string_by_id(this_module, local_module_atom_index);
*function_atom = module_get_atom_string_by_id(this_module, local_function_atom_index);
*module_atom = module_get_atom_string_by_id(this_module, local_module_atom_index, glb);
*function_atom = module_get_atom_string_by_id(this_module, local_function_atom_index, glb);
}
#endif

bool module_get_function_from_label(Module *this_module, int label, AtomString *function_name, int *arity)
bool module_get_function_from_label(Module *this_module, int label, AtomString *function_name, int *arity, GlobalContext *glb)
{
int best_label = -1;
const uint8_t *export_table_data = (const uint8_t *) this_module->export_table;
Expand All @@ -171,7 +171,7 @@ bool module_get_function_from_label(Module *this_module, int label, AtomString *
if (fun_label <= label && best_label < fun_label) {
best_label = fun_label;
*arity = fun_arity;
*function_name = module_get_atom_string_by_id(this_module, fun_atom_index);
*function_name = module_get_atom_string_by_id(this_module, fun_atom_index, glb);
}
}

Expand All @@ -184,7 +184,7 @@ bool module_get_function_from_label(Module *this_module, int label, AtomString *
if (fun_label <= label && best_label < fun_label) {
best_label = fun_label;
*arity = fun_arity;
*function_name = module_get_atom_string_by_id(this_module, fun_atom_index);
*function_name = module_get_atom_string_by_id(this_module, fun_atom_index, glb);
}
}
if (UNLIKELY(best_label == -1)) {
Expand All @@ -201,13 +201,13 @@ size_t module_get_exported_functions_count(Module *this_module)
return functions_count;
}

uint32_t module_search_exported_function(Module *this_module, AtomString func_name, int func_arity)
uint32_t module_search_exported_function(Module *this_module, AtomString func_name, int func_arity, GlobalContext *glb)
{
size_t functions_count = module_get_exported_functions_count(this_module);

const uint8_t *table_data = (const uint8_t *) this_module->export_table;
for (unsigned int i = 0; i < functions_count; i++) {
AtomString function_atom = module_get_atom_string_by_id(this_module, READ_32_ALIGNED(table_data + i * 12 + 12));
AtomString function_atom = module_get_atom_string_by_id(this_module, READ_32_ALIGNED(table_data + i * 12 + 12), glb);
int32_t arity = READ_32_ALIGNED(table_data + i * 12 + 4 + 12);
if ((func_arity == arity) && atom_are_equals(func_name, function_atom)) {
uint32_t label = READ_32_ALIGNED(table_data + i * 12 + 8 + 12);
Expand All @@ -218,17 +218,17 @@ uint32_t module_search_exported_function(Module *this_module, AtomString func_na
return 0;
}

term module_get_exported_functions(Module *this_module, Heap *heap, GlobalContext *global)
term module_get_exported_functions(Module *this_module, Heap *heap, GlobalContext *glb)
{
size_t functions_count = module_get_exported_functions_count(this_module);
term result_list = term_nil();

const uint8_t *table_data = (const uint8_t *) this_module->export_table;
for (unsigned int i = 0; i < functions_count; i++) {
AtomString function_atom = module_get_atom_string_by_id(this_module, READ_32_ALIGNED(table_data + i * 12 + 12));
AtomString function_atom = module_get_atom_string_by_id(this_module, READ_32_ALIGNED(table_data + i * 12 + 12), glb);
int32_t arity = READ_32_ALIGNED(table_data + i * 12 + 4 + 12);
term function_tuple = term_alloc_tuple(2, heap);
term_put_tuple_element(function_tuple, 0, globalcontext_existing_term_from_atom_string(global, function_atom));
term_put_tuple_element(function_tuple, 0, globalcontext_existing_term_from_atom_string(glb, function_atom));
term_put_tuple_element(function_tuple, 1, term_from_int(arity));
result_list = term_list_prepend(function_tuple, result_list, heap);
}
Expand Down Expand Up @@ -256,19 +256,18 @@ Module *module_new_from_iff_binary(GlobalContext *global, const void *iff_binary
memset(mod, 0, sizeof(Module));

mod->module_index = -1;
mod->global = global;

#ifndef AVM_NO_SMP
mod->mutex = smp_mutex_create();
#endif

if (UNLIKELY(module_populate_atoms_table(mod, beam_file + offsets[AT8U]) != MODULE_LOAD_OK)) {
if (UNLIKELY(module_populate_atoms_table(mod, beam_file + offsets[AT8U], global) != MODULE_LOAD_OK)) {
fprintf(stderr, "Error: Failed to populate atoms table: %s:%i.\n", __FILE__, __LINE__);
module_destroy(mod);
return NULL;
}

if (UNLIKELY(module_build_imported_functions_table(mod, beam_file + offsets[IMPT]) != MODULE_LOAD_OK)) {
if (UNLIKELY(module_build_imported_functions_table(mod, beam_file + offsets[IMPT], global) != MODULE_LOAD_OK)) {
fprintf(stderr, "Error: Failed to build imported functions table: %s:%i.\n", __FILE__, __LINE__);
module_destroy(mod);
return NULL;
Expand Down Expand Up @@ -410,17 +409,17 @@ term module_load_literal(Module *mod, int index, Context *ctx)
return t;
}

const struct ExportedFunction *module_resolve_function0(Module *mod, int import_table_index, struct UnresolvedFunctionCall *unresolved)
const struct ExportedFunction *module_resolve_function0(Module *mod, int import_table_index, struct UnresolvedFunctionCall *unresolved, GlobalContext *glb)
{

AtomString module_name_atom = (AtomString) valueshashtable_get_value(mod->global->atoms_ids_table, unresolved->module_atom_index, (unsigned long) NULL);
AtomString function_name_atom = (AtomString) valueshashtable_get_value(mod->global->atoms_ids_table, unresolved->function_atom_index, (unsigned long) NULL);
AtomString module_name_atom = (AtomString) valueshashtable_get_value(glb->atoms_ids_table, unresolved->module_atom_index, (unsigned long) NULL);
AtomString function_name_atom = (AtomString) valueshashtable_get_value(glb->atoms_ids_table, unresolved->function_atom_index, (unsigned long) NULL);
int arity = unresolved->arity;

Module *found_module = globalcontext_get_module(mod->global, module_name_atom);
Module *found_module = globalcontext_get_module(glb, module_name_atom);

if (LIKELY(found_module != NULL)) {
int exported_label = module_search_exported_function(found_module, function_name_atom, arity);
int exported_label = module_search_exported_function(found_module, function_name_atom, arity, glb);
if (exported_label == 0) {
char buf[256];
atom_write_mfa(buf, 256, module_name_atom, function_name_atom, arity);
Expand Down
19 changes: 10 additions & 9 deletions src/libAtomVM/module.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ struct LineRefOffset

struct Module
{
GlobalContext *global;

#ifdef ENABLE_ADVANCED_TRACE
void *import_table;
#endif
Expand Down Expand Up @@ -179,8 +177,9 @@ size_t module_get_exported_functions_count(Module *this_module);
* @param this_module the module on which the function will be searched.
* @param func_name function name atom string.
* @param func_arity function arity.
* @param glb the global context
*/
uint32_t module_search_exported_function(Module *this_module, AtomString func_name, int func_arity);
uint32_t module_search_exported_function(Module *this_module, AtomString func_name, int func_arity, GlobalContext *glb);

/**
* @brief Determine heap size of exported functions list.
Expand Down Expand Up @@ -242,10 +241,10 @@ term module_load_literal(Module *mod, int index, Context *ctx);
* @param local_atom_id module atom table index.
* @return the AtomString for the given module atom index.
*/
static inline AtomString module_get_atom_string_by_id(const Module *mod, int local_atom_id)
static inline AtomString module_get_atom_string_by_id(const Module *mod, int local_atom_id, GlobalContext *glb)
{
int global_id = mod->local_atoms_to_global_table[local_atom_id];
return (AtomString) valueshashtable_get_value(mod->global->atoms_ids_table, global_id, (unsigned long) NULL);
return (AtomString) valueshashtable_get_value(glb->atoms_ids_table, global_id, (unsigned long) NULL);
}

/**
Expand All @@ -262,7 +261,7 @@ static inline term module_get_atom_term_by_id(const Module *mod, int local_atom_
return term_from_atom_index(global_id);
}

const struct ExportedFunction *module_resolve_function0(Module *mod, int import_table_index, struct UnresolvedFunctionCall *unresolved);
const struct ExportedFunction *module_resolve_function0(Module *mod, int import_table_index, struct UnresolvedFunctionCall *unresolved, GlobalContext *glb);

/**
* @brief Get the module name, as an atom term.
Expand All @@ -284,8 +283,9 @@ static inline term module_get_name(const Module *mod)
* also it loads the referenced module if it hasn't been loaded yet.
* @param mod the module containing the function to resolve.
* @param import_table_index the unresolved function index.
* @param glb the global context
*/
static inline const struct ExportedFunction *module_resolve_function(Module *mod, int import_table_index)
static inline const struct ExportedFunction *module_resolve_function(Module *mod, int import_table_index, GlobalContext *glb)
{
SMP_MODULE_LOCK(mod);
// We cannot optimistically read the unresolved function call.
Expand All @@ -297,7 +297,7 @@ static inline const struct ExportedFunction *module_resolve_function(Module *mod
return func;
}
struct UnresolvedFunctionCall *unresolved = EXPORTED_FUNCTION_TO_UNRESOLVED_FUNCTION_CALL(func);
const struct ExportedFunction *result = module_resolve_function0(mod, import_table_index, unresolved);
const struct ExportedFunction *result = module_resolve_function0(mod, import_table_index, unresolved, glb);
SMP_MODULE_UNLOCK(mod);
return result;
}
Expand Down Expand Up @@ -370,8 +370,9 @@ static inline const uint8_t *module_get_str(Module *mod, size_t offset, size_t *
* @param label the current label used to look up the function/arity
* @param function_name (output) the function name, as an AtomString.
* @param arity (output) the function arity
* @param glb the global context
*/
bool module_get_function_from_label(Module *this_module, int label, AtomString *function_name, int *arity);
bool module_get_function_from_label(Module *this_module, int label, AtomString *function_name, int *arity, GlobalContext *glb);

/*
* @brief Insert the instruction offset for a given module at a line reference instruction.
Expand Down
4 changes: 2 additions & 2 deletions src/libAtomVM/nifs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,7 @@ static term nif_erlang_spawn(Context *ctx, int argc, term argv[])
if (UNLIKELY(!proper)) {
RAISE_ERROR(BADARG_ATOM);
}
int label = module_search_exported_function(found_module, function_string, args_len);
int label = module_search_exported_function(found_module, function_string, args_len, ctx->global);
//TODO: fail here if no function has been found
if (UNLIKELY(label == 0)) {
AVM_ABORT();
Expand Down Expand Up @@ -3091,7 +3091,7 @@ static term nif_erlang_function_exported(Context *ctx, int argc, term argv[])
return FALSE_ATOM;
}

int target_label = module_search_exported_function(target_module, function_name, arity);
int target_label = module_search_exported_function(target_module, function_name, arity, ctx->global);
if (target_label == 0) {
return FALSE_ATOM;
}
Expand Down
Loading

0 comments on commit 081289d

Please sign in to comment.