Skip to content

Commit

Permalink
ovsdb: Add lazy-copy support for ovsdb_datum objects.
Browse files Browse the repository at this point in the history 
Currently ovsdb-server is using shallow copies of some JSON objects
by keeping a reference counter.  JSON string objects are also used
directly as ovsdb atoms in database rows to avoid extra copies.

Taking this approach one step further ovsdb_datum objects can also
be mostly deduplicated by postponing the copy until it actually
needed.  datum object itself contains a type and 2 pointers to
data arrays.  Adding a one more pointer to a reference counter
we may create a shallow copy of the datum by simply copying type
and pointers and increasing the reference counter.

Before modifying the datum, special function needs to be called
to perform an actual copy of the object, a.k.a. unshare it.
Most of the datum modifications are performed inside the special
functions in ovsdb-data.c, so that is not very hard to track.
A few places like ovsdb-server.c and column mutations are accessing
and changing the data directly, so a few extra unshare() calls
has to be added there.

This change doesn't affect the maximum memory consumption too much,
because most of the copies are short-living.  However, not actually
performing these copies saves up to 40% of CPU time on operations
with large sets.

Reported-at: https://bugzilla.redhat.com/2069089
Acked-by: Dumitru Ceara <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
  • Loading branch information
@igsilya
igsilya committed Jul 13, 2022
1 parent 02dabb2 commit 485ac63
Show file tree
Hide file tree
Showing 13 changed files with 137 additions and 80 deletions.
4 changes: 2 additions & 2 deletions lib/db-ctl-base.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1503,7 +1503,7 @@ cmd_add(struct ctl_context *ctx)
}

type = &column->type;
ovsdb_datum_clone(&old, ovsdb_idl_read(row, column), &column->type);
ovsdb_datum_clone(&old, ovsdb_idl_read(row, column));
for (i = 4; i < ctx->argc; i++) {
struct ovsdb_type add_type;
struct ovsdb_datum add;
Expand Down Expand Up @@ -1588,7 +1588,7 @@ cmd_remove(struct ctl_context *ctx)
}

type = &column->type;
ovsdb_datum_clone(&old, ovsdb_idl_read(row, column), &column->type);
ovsdb_datum_clone(&old, ovsdb_idl_read(row, column));
for (i = 4; i < ctx->argc; i++) {
struct ovsdb_type rm_type;
struct ovsdb_datum rm;
Expand Down
117 changes: 86 additions & 31 deletions lib/ovsdb-data.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,7 @@ ovsdb_datum_init_empty(struct ovsdb_datum *datum)
datum->n = 0;
datum->keys = NULL;
datum->values = NULL;
datum->refcnt = NULL;
}

/* Initializes 'datum' as a datum that has the default value for 'type'.
Expand All @@ -917,6 +918,7 @@ ovsdb_datum_init_default(struct ovsdb_datum *datum,
datum->n = type->n_min;
datum->keys = alloc_default_atoms(type->key.type, datum->n);
datum->values = alloc_default_atoms(type->value.type, datum->n);
datum->refcnt = NULL;
}

/* Returns a read-only datum of the given 'type' that has the default value for
Expand All @@ -928,10 +930,12 @@ const struct ovsdb_datum *
ovsdb_datum_default(const struct ovsdb_type *type)
{
if (type->n_min == 0) {
static const struct ovsdb_datum empty;
static unsigned int refcnt = 1;
static const struct ovsdb_datum empty = { .refcnt = &refcnt };
return &empty;
} else if (type->n_min == 1) {
static struct ovsdb_datum default_data[OVSDB_N_TYPES][OVSDB_N_TYPES];
static unsigned int refcnt[OVSDB_N_TYPES][OVSDB_N_TYPES];
struct ovsdb_datum *d;
int kt = type->key.type;
int vt = type->value.type;
Expand All @@ -946,6 +950,8 @@ ovsdb_datum_default(const struct ovsdb_type *type)
d->values = CONST_CAST(union ovsdb_atom *,
ovsdb_atom_default(vt));
}
d->refcnt = &refcnt[kt][vt];
*d->refcnt = 1;
}
return d;
} else {
Expand Down Expand Up @@ -999,18 +1005,23 @@ clone_atoms(const union ovsdb_atom *old, enum ovsdb_atomic_type type, size_t n)
}
}

/* Initializes 'new' as a copy of 'old', with the given 'type'.
/* Initializes 'new' as a shallow copy of 'old_'.
*
* The caller must eventually arrange for 'new' to be destroyed (with
* ovsdb_datum_destroy()). */
* ovsdb_datum_destroy()). The caller must call ovsdb_datum_unshare()
* before attempting direct modifications of the 'new' or 'old_', i.e.
* modifications outside of the ovsdb_datum_* API. */
void
ovsdb_datum_clone(struct ovsdb_datum *new, const struct ovsdb_datum *old,
const struct ovsdb_type *type)
ovsdb_datum_clone(struct ovsdb_datum *new, const struct ovsdb_datum *old_)
{
unsigned int n = old->n;
new->n = n;
new->keys = clone_atoms(old->keys, type->key.type, n);
new->values = clone_atoms(old->values, type->value.type, n);
struct ovsdb_datum *old = CONST_CAST(struct ovsdb_datum *, old_);

if (!old->refcnt) {
old->refcnt = xmalloc(sizeof *old->refcnt);
*old->refcnt = 1;
}
memcpy(new, old, sizeof *new);
(*new->refcnt)++;
}

static void
Expand All @@ -1037,8 +1048,25 @@ free_data(enum ovsdb_atomic_type type,
void
ovsdb_datum_destroy(struct ovsdb_datum *datum, const struct ovsdb_type *type)
{
free_data(type->key.type, datum->keys, datum->n);
free_data(type->value.type, datum->values, datum->n);
if (!datum->refcnt || !--(*datum->refcnt)) {
free_data(type->key.type, datum->keys, datum->n);
free_data(type->value.type, datum->values, datum->n);
free(datum->refcnt);
}
}

/* This function should be called before attempting direct modifications
* of the 'datum', i.e. modifications outside of the ovsdb_datum_* API. */
void
ovsdb_datum_unshare(struct ovsdb_datum *datum, const struct ovsdb_type *type)
{
if (!datum->refcnt || *datum->refcnt == 1) {
return;
}
datum->keys = clone_atoms(datum->keys, type->key.type, datum->n);
datum->values = clone_atoms(datum->values, type->value.type, datum->n);
(*datum->refcnt)--;
datum->refcnt = NULL;
}

/* Swaps the contents of 'a' and 'b', which need not have the same type. */
Expand Down Expand Up @@ -1107,19 +1135,21 @@ ovsdb_datum_sort__(struct ovsdb_datum *datum, enum ovsdb_atomic_type key_type,
* caller must free the returned error when it is no longer needed. On error,
* 'datum' is sorted but not unique. */
struct ovsdb_error *
ovsdb_datum_sort(struct ovsdb_datum *datum, enum ovsdb_atomic_type key_type)
ovsdb_datum_sort(struct ovsdb_datum *datum, const struct ovsdb_type *type)
{
size_t i;

if (datum->n < 2) {
return NULL;
}

ovsdb_datum_sort__(datum, key_type, OVSDB_TYPE_VOID);
ovsdb_datum_unshare(datum, type);

ovsdb_datum_sort__(datum, type->key.type, OVSDB_TYPE_VOID);

for (i = 0; i < datum->n - 1; i++) {
if (ovsdb_atom_equals(&datum->keys[i], &datum->keys[i + 1],
key_type)) {
type->key.type)) {
if (datum->values) {
return ovsdb_error(NULL, "map contains duplicate key");
} else {
Expand All @@ -1135,9 +1165,9 @@ ovsdb_datum_sort(struct ovsdb_datum *datum, enum ovsdb_atomic_type key_type)
* this function assert-fails if it actually does. */
void
ovsdb_datum_sort_assert(struct ovsdb_datum *datum,
enum ovsdb_atomic_type key_type)
const struct ovsdb_type *type)
{
struct ovsdb_error *error = ovsdb_datum_sort(datum, key_type);
struct ovsdb_error *error = ovsdb_datum_sort(datum, type);
if (error) {
OVS_NOT_REACHED();
}
Expand All @@ -1150,29 +1180,29 @@ ovsdb_datum_sort_assert(struct ovsdb_datum *datum,
* Returns the number of keys (or pairs) that were dropped. */
size_t
ovsdb_datum_sort_unique(struct ovsdb_datum *datum,
enum ovsdb_atomic_type key_type,
enum ovsdb_atomic_type value_type)
const struct ovsdb_type *type)
{
size_t src, dst;

if (datum->n < 2) {
return 0;
}

ovsdb_datum_sort__(datum, key_type, value_type);
ovsdb_datum_unshare(datum, type);
ovsdb_datum_sort__(datum, type->key.type, type->value.type);

dst = 1;
for (src = 1; src < datum->n; src++) {
if (ovsdb_atom_equals(&datum->keys[src], &datum->keys[dst - 1],
key_type)) {
ovsdb_atom_destroy(&datum->keys[src], key_type);
if (value_type != OVSDB_TYPE_VOID) {
ovsdb_atom_destroy(&datum->values[src], value_type);
type->key.type)) {
ovsdb_atom_destroy(&datum->keys[src], type->key.type);
if (type->value.type != OVSDB_TYPE_VOID) {
ovsdb_atom_destroy(&datum->values[src], type->value.type);
}
} else {
if (src != dst) {
datum->keys[dst] = datum->keys[src];
if (value_type != OVSDB_TYPE_VOID) {
if (type->value.type != OVSDB_TYPE_VOID) {
datum->values[dst] = datum->values[src];
}
}
Expand Down Expand Up @@ -1258,6 +1288,7 @@ ovsdb_datum_from_json__(struct ovsdb_datum *datum,
datum->n = 0;
datum->keys = xmalloc(n * sizeof *datum->keys);
datum->values = is_map ? xmalloc(n * sizeof *datum->values) : NULL;
datum->refcnt = NULL;
for (i = 0; i < n; i++) {
const struct json *element = inner->array.elems[i];
const struct json *key = NULL;
Expand Down Expand Up @@ -1298,6 +1329,7 @@ ovsdb_datum_from_json__(struct ovsdb_datum *datum,
datum->n = 1;
datum->keys = xmalloc(sizeof *datum->keys);
datum->values = NULL;
datum->refcnt = NULL;

error = ovsdb_atom_from_json(&datum->keys[0], &type->key,
json, symtab);
Expand Down Expand Up @@ -1331,7 +1363,7 @@ ovsdb_datum_from_json(struct ovsdb_datum *datum,
return error;
}

error = ovsdb_datum_sort(datum, type->key.type);
error = ovsdb_datum_sort(datum, type);
if (error) {
ovsdb_datum_destroy(datum, type);
}
Expand Down Expand Up @@ -1609,7 +1641,7 @@ ovsdb_datum_from_string(struct ovsdb_datum *datum,
goto error;
}

dberror = ovsdb_datum_sort(datum, type->key.type);
dberror = ovsdb_datum_sort(datum, type);
if (dberror) {
ovsdb_error_destroy(dberror);
if (ovsdb_type_is_map(type)) {
Expand Down Expand Up @@ -1687,6 +1719,7 @@ ovsdb_datum_from_smap(struct ovsdb_datum *datum, const struct smap *smap)
datum->n = smap_count(smap);
datum->keys = xmalloc(datum->n * sizeof *datum->keys);
datum->values = xmalloc(datum->n * sizeof *datum->values);
datum->refcnt = NULL;

struct smap_node *node;
size_t i = 0;
Expand All @@ -1697,7 +1730,11 @@ ovsdb_datum_from_smap(struct ovsdb_datum *datum, const struct smap *smap)
}
ovs_assert(i == datum->n);

ovsdb_datum_sort_unique(datum, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING);
struct ovsdb_type type = {
OVSDB_BASE_STRING_INIT, OVSDB_BASE_STRING_INIT,
0, UINT_MAX
};
ovsdb_datum_sort_unique(datum, &type);
}

struct ovsdb_error * OVS_WARN_UNUSED_RESULT
Expand Down Expand Up @@ -1774,6 +1811,10 @@ ovsdb_datum_compare_3way(const struct ovsdb_datum *a,
return a->n < b->n ? -1 : 1;
}

if (a->refcnt && a->refcnt == b->refcnt) {
return 0;
}

cmp = atom_arrays_compare_3way(a->keys, b->keys, type->key.type, a->n);
if (cmp) {
return cmp;
Expand Down Expand Up @@ -1893,6 +1934,8 @@ static void
ovsdb_datum_reallocate(struct ovsdb_datum *a, const struct ovsdb_type *type,
unsigned int capacity)
{
ovsdb_datum_unshare(a, type);

a->keys = xrealloc(a->keys, capacity * sizeof *a->keys);
if (type->value.type != OVSDB_TYPE_VOID) {
a->values = xrealloc(a->values, capacity * sizeof *a->values);
Expand All @@ -1909,6 +1952,8 @@ void
ovsdb_datum_remove_unsafe(struct ovsdb_datum *datum, size_t idx,
const struct ovsdb_type *type)
{
ovsdb_datum_unshare(datum, type);

ovsdb_atom_destroy(&datum->keys[idx], type->key.type);
datum->keys[idx] = datum->keys[datum->n - 1];
if (type->value.type != OVSDB_TYPE_VOID) {
Expand Down Expand Up @@ -1940,6 +1985,9 @@ ovsdb_datum_add_unsafe(struct ovsdb_datum *datum,
const union ovsdb_atom *range_end_atom)
{
size_t idx = datum->n;

ovsdb_datum_unshare(datum, type);

datum->n += range_end_atom ?
(range_end_atom->integer - key->integer + 1) : 1;
datum->keys = xrealloc(datum->keys, datum->n * sizeof *datum->keys);
Expand Down Expand Up @@ -1984,6 +2032,8 @@ ovsdb_datum_push_unsafe(struct ovsdb_datum *dst,
return;
}

ovsdb_datum_unshare(dst, type);

memcpy(&dst->keys[dst->n], &src->keys[start_idx], n * sizeof src->keys[0]);
if (type->value.type != OVSDB_TYPE_VOID) {
memcpy(&dst->values[dst->n], &src->values[start_idx],
Expand All @@ -1999,6 +2049,7 @@ ovsdb_datum_union(struct ovsdb_datum *a, const struct ovsdb_datum *b,
struct ovsdb_datum result;
unsigned int copied, pos;

ovsdb_datum_unshare(a, type);
ovsdb_datum_init_empty(&result);

copied = 0;
Expand Down Expand Up @@ -2050,6 +2101,8 @@ ovsdb_datum_subtract(struct ovsdb_datum *a, const struct ovsdb_type *a_type,
ovs_assert(a_type->value.type == b_type->value.type
|| b_type->value.type == OVSDB_TYPE_VOID);

ovsdb_datum_unshare(a, a_type);

idx = xmalloc(b->n * sizeof *idx);
n_idx = 0;
for (size_t bi = 0; bi < b->n; bi++) {
Expand Down Expand Up @@ -2168,8 +2221,8 @@ ovsdb_datum_added_removed(struct ovsdb_datum *added,
ovsdb_datum_init_empty(added);
ovsdb_datum_init_empty(removed);
if (!ovsdb_type_is_composite(type)) {
ovsdb_datum_clone(removed, old, type);
ovsdb_datum_clone(added, new, type);
ovsdb_datum_clone(removed, old);
ovsdb_datum_clone(added, new);
return;
}

Expand Down Expand Up @@ -2228,7 +2281,7 @@ ovsdb_datum_diff(struct ovsdb_datum *diff,

ovsdb_datum_init_empty(diff);
if (!ovsdb_type_is_composite(type)) {
ovsdb_datum_clone(diff, new, type);
ovsdb_datum_clone(diff, new);
return;
}

Expand Down Expand Up @@ -2283,10 +2336,12 @@ ovsdb_datum_apply_diff_in_place(struct ovsdb_datum *a,

if (!ovsdb_type_is_composite(type)) {
ovsdb_datum_destroy(a, type);
ovsdb_datum_clone(a, diff, type);
ovsdb_datum_clone(a, diff);
return NULL;
}

ovsdb_datum_unshare(a, type);

operation = xmalloc(diff->n * sizeof *operation);
idx = xmalloc(diff->n * sizeof *idx);
new_size = a->n;
Expand Down
13 changes: 7 additions & 6 deletions lib/ovsdb-data.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ struct ovsdb_datum {
unsigned int n; /* Number of 'keys' and 'values'. */
union ovsdb_atom *keys; /* Each of the ovsdb_type's 'key_type'. */
union ovsdb_atom *values; /* Each of the ovsdb_type's 'value_type'. */
unsigned int *refcnt; /* Number of copies with the same
* 'keys' and 'values'. */
};
#define OVSDB_DATUM_INITIALIZER { 0, NULL, NULL }

Expand All @@ -155,22 +157,21 @@ void ovsdb_datum_init_default(struct ovsdb_datum *, const struct ovsdb_type *);
bool ovsdb_datum_is_default(const struct ovsdb_datum *,
const struct ovsdb_type *);
const struct ovsdb_datum *ovsdb_datum_default(const struct ovsdb_type *);
void ovsdb_datum_clone(struct ovsdb_datum *, const struct ovsdb_datum *,
const struct ovsdb_type *);
void ovsdb_datum_clone(struct ovsdb_datum *, const struct ovsdb_datum *);
void ovsdb_datum_destroy(struct ovsdb_datum *, const struct ovsdb_type *);
void ovsdb_datum_unshare(struct ovsdb_datum *, const struct ovsdb_type *);
void ovsdb_datum_swap(struct ovsdb_datum *, struct ovsdb_datum *);

/* Checking and maintaining invariants. */
struct ovsdb_error *ovsdb_datum_sort(struct ovsdb_datum *,
enum ovsdb_atomic_type key_type)
const struct ovsdb_type *type)
OVS_WARN_UNUSED_RESULT;

void ovsdb_datum_sort_assert(struct ovsdb_datum *,
enum ovsdb_atomic_type key_type);
const struct ovsdb_type *type);

size_t ovsdb_datum_sort_unique(struct ovsdb_datum *,
enum ovsdb_atomic_type key_type,
enum ovsdb_atomic_type value_type);
const struct ovsdb_type *type);

struct ovsdb_error *ovsdb_datum_check_constraints(
const struct ovsdb_datum *, const struct ovsdb_type *)
Expand Down
Loading

0 comments on commit 485ac63

Please sign in to comment.