Skip to content

Commit

Permalink
Remove unused global storage in shared memory
Browse files Browse the repository at this point in the history
Summary: The only place that initializes shared memory always sets the size of this to 0.

Reviewed By: yangdanny97

Differential Revision: D57416763

fbshipit-source-id: c27dc39ebf0ff7a632560abadb261ae5678ea3ce
  • Loading branch information
samwgoldman authored and facebook-github-bot committed May 17, 2024
1 parent fb27c8e commit 8e37926
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 91 deletions.
89 changes: 10 additions & 79 deletions source/hack_parallel/hack_parallel/heap/hh_shared.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,16 @@
* The lock-free data structures implemented here only work because of how
* the Hack phases are synchronized.
*
* There are 3 kinds of storage implemented in this file.
* I) The global storage. Used by the master to efficiently transfer a blob
* of data to the workers. This is used to share an environment in
* read-only mode with all the workers.
* The master stores, the workers read.
* Only concurrent reads allowed. No concurrent write/read and write/write.
* There are a few different OCaml modules that act as interfaces to this
* global storage. They all use the same area of memory, so only one can be
* active at any one time. The first word indicates the size of the global
* storage currently in use; callers are responsible for setting it to zero
* once they are done.
*
* II) The dependency table. It's a hashtable that contains all the
* There are 2 kinds of storage implemented in this file.
* I) The dependency table. It's a hashtable that contains all the
* dependencies between Hack objects. It is filled concurrently by
* the workers. The dependency table is made of 2 hashtables, one that
* is used to quickly answer if a dependency exists. The other one
* to retrieve the list of dependencies associated with an object.
* Only the hashes of the objects are stored, so this uses relatively
* little memory. No dynamic allocation is required.
*
* III) The hashtable that maps string keys to string values. (The strings
* II) The hashtable that maps string keys to string values. (The strings
* are really serialized / marshalled representations of OCaml structures.)
* Key observation of the table is that data with the same key are
* considered equivalent, and so you can arbitrarily get any copy of it;
Expand Down Expand Up @@ -71,7 +60,7 @@
* Since the values are variably sized and can get quite large, they are
* stored separately from the hashes in a garbage-collected heap.
*
* Both II and III resolve hash collisions via linear probing.
* Both I and II resolve hash collisions via linear probing.
*/
/*****************************************************************************/

Expand Down Expand Up @@ -148,7 +137,6 @@ static sqlite3_stmt* get_select_stmt = NULL;

/* Convention: .*_b = Size in bytes. */

static size_t global_size_b;
static size_t heap_size;

/* Used for the dependency hashtable */
Expand Down Expand Up @@ -241,11 +229,6 @@ static size_t shared_mem_size = 0;
/* Beginning of shared memory */
static char* shared_mem = NULL;

/* ENCODING: The first element is the size stored in bytes, the rest is
* the data. The size is set to zero when the storage is empty.
*/
static value* global_storage = NULL;

/* A pair of a 31-bit unsigned number and a tag bit. */
typedef struct {
uint32_t num : 31;
Expand Down Expand Up @@ -543,10 +526,6 @@ static void define_globals(size_t page_size) {
mem += page_size;
/* END OF THE SMALL OBJECTS PAGE */

/* Global storage initialization */
global_storage = (value*)mem;
mem += global_size_b;

/* Dependencies */
deptbl = (deptbl_entry_t*)mem;
mem += dep_size_b;
Expand Down Expand Up @@ -590,8 +569,6 @@ static void init_zstd_compression() {
}

static void init_shared_globals(size_t page_size, size_t config_log_level) {
// Initial size is zero for global storage is zero
global_storage[0] = 0;
// Initialize the number of element in the table
*hcounter = 0;
*dcounter = 0;
Expand All @@ -617,24 +594,23 @@ CAMLprim value hh_shared_init(value config_val) {
CAMLparam1(config_val);
size_t page_size = getpagesize();

global_size_b = Long_val(Field(config_val, 0));
heap_size = Long_val(Field(config_val, 1));
dep_size = 1ul << Long_val(Field(config_val, 2));
heap_size = Long_val(Field(config_val, 0));
dep_size = 1ul << Long_val(Field(config_val, 1));
dep_size_b = dep_size * sizeof(deptbl[0]);
bindings_size_b = dep_size * sizeof(deptbl_bindings[0]);
hashtbl_size = 1ul << Long_val(Field(config_val, 3));
hashtbl_size = 1ul << Long_val(Field(config_val, 2));
hashtbl_size_b = hashtbl_size * sizeof(hashtbl[0]);

shared_mem_size = global_size_b + dep_size_b + bindings_size_b +
hashtbl_size_b + heap_size + 3 * page_size;
shared_mem_size =
dep_size_b + bindings_size_b + hashtbl_size_b + heap_size + 3 * page_size;

define_globals(page_size);

// Keeping the pids around to make asserts.
*master_pid = getpid();
my_pid = *master_pid;

init_shared_globals(page_size, Long_val(Field(config_val, 4)));
init_shared_globals(page_size, Long_val(Field(config_val, 3)));
// Checking that we did the maths correctly.
assert(*heap + heap_size == shared_mem + shared_mem_size);

Expand All @@ -661,9 +637,6 @@ value hh_connect(value unit) {
}

void pyre_reset() {
// Reset global storage
global_storage[0] = 0;

// Reset the number of element in the table
*hcounter = 0;
*dcounter = 0;
Expand Down Expand Up @@ -763,48 +736,6 @@ CAMLprim value hh_assert_allow_dependency_table_reads(void) {
CAMLreturn(Val_unit);
}

/*****************************************************************************/
/* Global storage */
/*****************************************************************************/

void hh_shared_store(value data) {
CAMLparam1(data);
size_t size = caml_string_length(data);

assert_master(); // only the master can store
assert(global_storage[0] == 0); // Is it clear?
assert(size < global_size_b - sizeof(value)); // Do we have enough space?

global_storage[0] = size;
memcpy(&global_storage[1], String_val(data), size);

CAMLreturn0;
}

/*****************************************************************************/
/* We are allocating ocaml values. The OCaml GC must know about them.
* caml_alloc_string might trigger the GC, when that happens, the GC needs
* to scan the stack to find the OCaml roots. The macros CAMLparam0 and
* CAMLlocal1 register the roots.
*/
/*****************************************************************************/

CAMLprim value hh_shared_load(void) {
CAMLparam0();
CAMLlocal1(result);

size_t size = global_storage[0];
assert(size != 0);
result = caml_alloc_initialized_string(size, (char*)&global_storage[1]);

CAMLreturn(result);
}

void hh_shared_clear(void) {
assert_master();
global_storage[0] = 0;
}

/*****************************************************************************/
/* Dependencies */
/*****************************************************************************/
Expand Down
7 changes: 0 additions & 7 deletions source/hack_parallel/hack_parallel/heap/hh_shared.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,6 @@ CAMLprim value hh_hash_slots(void);
*/
CAMLprim value hh_counter_next(void);

/*****************************************************************************/
/* Global storage. */
/*****************************************************************************/
void hh_shared_store(value data);
CAMLprim value hh_shared_load(void);
void hh_shared_clear(void);

/*****************************************************************************/
/* Garbage collection. */
/*****************************************************************************/
Expand Down
1 change: 0 additions & 1 deletion source/hack_parallel/hack_parallel/heap/sharedMemory.ml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ module Measure = Hack_utils.Measure
(* Don't change the ordering of this record without updating hh_shared_init in
* hh_shared.c, which indexes into config objects *)
type config = {
global_size : int;
heap_size : int;
dep_table_pow : int;
hash_table_pow : int;
Expand Down
1 change: 0 additions & 1 deletion source/hack_parallel/hack_parallel/heap/sharedMemory.mli
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
(*****************************************************************************)

type config = {
global_size : int;
heap_size : int;
dep_table_pow : int;
hash_table_pow : int;
Expand Down
4 changes: 1 addition & 3 deletions source/service/memory.ml
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ let initialize ~heap_size ~dep_table_pow ~hash_table_pow ~log_level () =
allocation_policy = best_fit_allocation_policy;
space_overhead = 120;
};
let shared_mem_config =
{ SharedMemory.global_size = 0; heap_size; dep_table_pow; hash_table_pow; log_level }
in
let shared_mem_config = { SharedMemory.heap_size; dep_table_pow; hash_table_pow; log_level } in
Log.info
"Initializing shared memory (heap_size: %d, dep_table_pow: %d, hash_table_pow: %d)"
heap_size
Expand Down

0 comments on commit 8e37926

Please sign in to comment.