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

RFC: Add a persistent term storage #1989

Merged
merged 5 commits into from Nov 6, 2018

Conversation

@bjorng
Copy link
Contributor

bjorng commented Oct 17, 2018

Persistent terms are useful for storing Erlang terms that are never
or infrequently updated. They have the following advantages:

  • Constant time access. A persistent term is not copied when it is
    looked up. The constant factor is lower than for ETS, and no locks
    are taken when looking up a term.

  • Persistent terms are not copied in garbage collections.

  • There is only ever one copy of a persistent term (until it is
    deleted). That makes them useful for storing configuration data
    that needs to be easily accessible by all processes.

Persistent terms have the following drawbacks:

  • Updates are expensive. The hash table holding the keys for the
    persistent terms are updated whenever a persistent term is added,
    updated or deleted.

  • Updating or deleting a persistent term triggers a "global GC", which
    will schedule a heap scan of all processes to search the heap of all
    processes for the deleted term. If a process still holds a reference
    to the deleted term, the process will be garbage collected and the
    term copied to the heap of the process. This global GC can make the
    system less responsive for some time.

Three BIFs (implemented in C in the emulator) is the entire
interface to the persistent term functionality:

  • put(Key, Value) to store a persistent term.

  • get(Key) to look up a persistent term.

  • erase(Key) to delete a persistent term.

There are also two additional BIFs to obtain information about
persistent terms:

  • info() to return a map with information about persistent terms.

  • get() to return a list of a {Key,Value} tuples for all persistent
    terms. (The values are not copied, but the keys are.)

@bjorng bjorng self-assigned this Oct 17, 2018

@bjorng bjorng requested review from garazdawi , sverker and rickard-green Oct 17, 2018

@essen

This comment has been minimized.

Copy link
Contributor

essen commented Oct 17, 2018

Sounds like it would be perfect for Cowboy's routing data, which rarely changes, can be fairly big, is passed on to every request process when they spawn, and from which only a small term is looked up to process said request. Passing on the persistent_term key instead of the whole thing would reduce the footprint substantially.

@fogfish

This comment has been minimized.

Copy link

fogfish commented Oct 17, 2018

Will it support functions as terms or it is limited to tuples, maps, lists and others?

put(foo, fun() -> bar end).
@bjorng

This comment has been minimized.

Copy link
Contributor

bjorng commented Oct 17, 2018

Yes, all term types (including funs) are supported.

@ferd

This comment has been minimized.

Copy link
Contributor

ferd commented Oct 17, 2018

The global GC is a bit annoying but it seems unavoidable. Seems like a good way to kill patterns like mochiglobals and other "live-compiled modules as data stores" out there, which represented a legitimate need. I wouldn't imagine it displacing ETS or pdicts or anything like that in usage either ideally.

@okeuday

This comment has been minimized.

Copy link
Contributor

okeuday commented Oct 18, 2018

@bjorng The functionality here looks very useful. I have two suggestions:

  • Since the data is global to all processes, it would be helpful if a Table function argument (or a different name, but the same idea) would be added as the first argument to every function, so that different Erlang applications won't impact each other in erroneous ways. The addition would establish scope for the data storage (and avoid a problem that is present in modules like pg2).
  • Due to updates being expensive with individual function calls multiplying the problem, it would be best to have a function that accepts a list of {Key, Value} tuples to set in a single function call. That function would help minimize the latency as the mostly constant data changes.

Happy to see the idea in Erlang/OTP. Having this will help to avoid the mochiglobal kludge or strange misuses of large binaries (which are rough on GC, unless the binaries are never changed).

@jhogberg

This comment has been minimized.

Copy link
Contributor

jhogberg commented Oct 18, 2018

Since the data is global to all processes, it would be helpful if a Table function argument (or a different name, but the same idea) would be added as the first argument to every function, so that different Erlang applications won't impact each other in erroneous ways. The addition would establish scope for the data storage (and avoid a problem that is present in modules like pg2).

Looking up complex keys is more expensive than simple ones and having a "namespace" argument would mean all keys need to be complex. We don't want to force this cost on everyone, and we don't see this as a big problem as collisions are easily avoided by following the recommendation to use ?MODULE or {?MODULE, Key} as keys.

Due to updates being expensive with individual function calls multiplying the problem, it would be best to have a function that accepts a list of {Key, Value} tuples to set in a single function call. That function would help minimize the latency as the mostly constant data changes.

This would not improve latency to any meaningful degree. If you have terms that are updated together we recommend storing them as a map or tuple instead.

Persistent terms should not be your first choice for term storage. If you expect to change the values you should take a long hard look at ETS first.

@bjorng bjorng force-pushed the bjorng:bjorn/erts/persistent_terms/OTP-14669 branch from 0289977 to a0b336e Oct 19, 2018

@okeuday

This comment has been minimized.

Copy link
Contributor

okeuday commented Oct 20, 2018

@bjorng @jhogberg

Looking up complex keys is more expensive than simple ones and having a "namespace" argument would mean all keys need to be complex. We don't want to force this cost on everyone, and we don't see this as a big problem as collisions are easily avoided by following the recommendation to use ?MODULE or {?MODULE, Key} as keys.

Ok, thanks for putting this into the documentation, this makes the usage clear.

Persistent terms should not be your first choice for term storage. If you expect to change the values you should take a long hard look at ETS first.

My only other suggestion, is that the name persistent_term is misleading because it could be understood to relate to persisting data to disk, which is a common interpretation of persistence as a concept. Instead, it seems better to use the name constants or constant, though you may also think that name misleading because the data is allowed to change infrequently.

The name constants or constant would refer both to the usage of the data as a storage for constants and would promote usage that rarely changes. Ideally, the usage would be for data that only changes during a module hot-update, which I interpret as the main intent for allowing the data to change after it has been stored. The name constant would make the use-case clear and could also refer to the constant-time access characteristics with its data, so it would have 2 clear connotations. The alternative name constant_pool may be more accurate, though constants/constant is both shorter and simpler than constant_pool/persistent_term.

If the name changes to use the word constant it seems more likely that this functionality would see less misuse than it might otherwise experience and it avoids any misunderstanding of the word persistent.

@fogfish

This comment has been minimized.

Copy link

fogfish commented Oct 21, 2018

In computing, a persistent data structure is a data structure that always preserves the previous version of itself when it is modified. Such data structures are effectively immutable, as their operations do not (visibly) update the structure in-place, but instead always yield a new updated structure.

In this respect, persistent should not bother too much. The constant has completely different meaning then semantic exposed by this api.

@bjorng

This comment has been minimized.

Copy link
Contributor

bjorng commented Oct 21, 2018

@okeuday We did consider using constant but rejected it because we felt it was too confusing. Naming is one of the hardest problems in computer science (I won't name the other hard problems out of fear of making an off-by-one error). We finally settled on persistent because it seemed to be the least confusing of the names we considered.

@bjorng bjorng force-pushed the bjorng:bjorn/erts/persistent_terms/OTP-14669 branch from a0b336e to 07ef616 Oct 22, 2018

@OvermindDL1

This comment has been minimized.

Copy link

OvermindDL1 commented Oct 22, 2018

Isn't it more accurate to call them interned? Perhaps that should be used?

@vans163

This comment has been minimized.

Copy link
Contributor

vans163 commented Oct 23, 2018

I jumped but then realized this has no relation to state machine replication / persistanceactors (like in scala).

BTW is distributed Erlang in the picture?

@garazdawi
Copy link
Contributor

garazdawi left a comment

Naming the feature has been the most problematic part. For a long time I was of the opinion that since we could not find a good name to describe what it is that we are doing, we should not do it. Or possibly put it into an already existing API that does the same thing.

Some crazy ideas that I came up with along the way are:

  • to view it as a global process dictionary and put the API in erlang:put_term/2.
  • to view it as a specialized ets table and put an option at table creation about the properties
  • erts:put/2, Erlang Restricted Term Storage
  • pets:put/2, Persistent Erlang Term Storage
  • lets:put/2, Literal Erlang Term Storage

persistent_term is not a great name for this, because of it's associations with persisting to disk and to immutability. It is however in my oppionion better than the other suggestions, and this functionality is needed as shown by the usage of mochiglobal and similar libraries.

I don't think we have considered interned_term before, that may be a better name.

@vans163 This is a local optimization and has nothing to do with distributed Erlang.

Show resolved Hide resolved erts/doc/src/persistent_term.xml Outdated
allocator also used for literals (constant terms) in BEAM code.
By default, 1 GB of virtual address space is reserved for literals
in BEAM code and persistent terms. The amount of virtual address
space reserved for literals can be changed by using the <c>MIscs</c>

This comment has been minimized.

@garazdawi

garazdawi Oct 23, 2018

Contributor

+MIscs ? Link to docs?

This comment has been minimized.

@bjorng

bjorng Oct 24, 2018

Contributor

Will fix.

Eterm res = NIL;

trap_data = (TrapData *) hp;
trap_data->header = make_pos_bignum_header(trap_data_size-1);

This comment has been minimized.

@garazdawi

garazdawi Oct 23, 2018

Contributor

Creative way to create a trap context :)

Show resolved Hide resolved erts/emulator/beam/erl_bif_persistent.c Outdated
Show resolved Hide resolved erts/emulator/beam/erl_bif_persistent.c
Show resolved Hide resolved erts/emulator/test/persistent_term_SUITE.erl
@OvermindDL1

This comment has been minimized.

Copy link

OvermindDL1 commented Oct 23, 2018

Hmm, actually interning is generally implicit and automatic, the explicit version of interning is generally called a flyweight value (in C++-land anyway). Flyweight might be the most accurate, but I still prefer interned...

@okeuday

This comment has been minimized.

Copy link
Contributor

okeuday commented Oct 23, 2018

I am not keen on the interned word usage due to associations the word has (Erlang already kills children processes, but now it wants to put data into internment camps?).

With constant being considered too confusing, static may be closest to that idea without inferring the data stored is constant (similar to C/C++ usage of static for global data that is ideally constant). Otherwise frozen may also work as a module name.

Anyway, not trying to cause any interruption in the merge of this functionality. If persistent_term is best, that works. The functionality is what is important.

@kjnilsson

This comment has been minimized.

Copy link
Contributor

kjnilsson commented Oct 23, 2018

inert term storage / its ;)

@OvermindDL1

This comment has been minimized.

Copy link

OvermindDL1 commented Oct 23, 2018

I am not keen on the interned word usage due to associations the word has (Erlang already kills children processes, but now it wants to put data into internment camps?).

Heh, oh wow, never associated that before (I tend to know the programming-related meanings of things)... >.>

Extend the sharing-preserving routines to optionally copy literals
In the implementation of the zero-copying term storage, we
want to preserve sharing, but not copy literals because the
modules holding the literals could be unloaded under our feet.

@bjorng bjorng force-pushed the bjorng:bjorn/erts/persistent_terms/OTP-14669 branch from 07ef616 to 0161799 Oct 24, 2018

@bullno1

This comment has been minimized.

Copy link

bullno1 commented Oct 24, 2018

erts:put/2, Erlang Restricted Term Storage
pets:put/2, Persistent Erlang Term Storage
lets:put/2, Literal Erlang Term Storage

ERTS is already Erlang Runtime System
Pets sound weird
There is already a project called lets and let's not talk about namespaces for now.

How about cdb? Or ecdb? At least the term cdb already has some pre-existing use: Something that is quick to lookup but slow to modify because modification is basically rebuilding.

new_table = (HashTable *) data;
ASSERT(new_table->num_to_delete == 0);
erts_atomic_set_nob(&the_hash_table, (erts_aint_t)new_table);
erts_schedule_thr_prgr_later_op(table_deleter,

This comment has been minimized.

@sverker

sverker Oct 24, 2018

Contributor

It seems safe to call release_update_permission(1) here directly after the_hash_table is set. table_deleter is a pure cleanup that no one needs to wait for.

This comment has been minimized.

@bjorng

bjorng Oct 26, 2018

Contributor

Fixed.

Show resolved Hide resolved erts/emulator/beam/erl_bif_persistent.c
Show resolved Hide resolved erts/emulator/beam/erl_bif_persistent.c Outdated
Show resolved Hide resolved erts/emulator/beam/erl_bif_persistent.c
Show resolved Hide resolved erts/emulator/beam/erl_bif_persistent.c Outdated
Show resolved Hide resolved erts/emulator/beam/erl_bif_persistent.c
}

static HashTable*
copy_table(HashTable* old_table, Uint new_size, int rehash)

This comment has been minimized.

@garazdawi

garazdawi Oct 24, 2018

Contributor

How large do we expect this hash table to get? Do we need to trap while copying it?

This comment has been minimized.

@bjorng

bjorng Oct 25, 2018

Contributor

The recommendation in the documentation is to limit the total number of persistent terms. Therefore, I have assumed that the table is always small enough so that we don't have to trap.


ASSERT(is_tuple_arity(old_term, 2));
old_table->term[entry_index] = NIL;
old_table->num_entries--;

This comment has been minimized.

@sverker

sverker Oct 24, 2018

Contributor

Here in erase/1 you write into the active old_table. I think this can lead to all kinds of buggy behavior with concurrent readers like get/0.
Instead treat old_table as read only, copy it and make changes in new_table.

This comment has been minimized.

@bjorng

bjorng Oct 25, 2018

Contributor

Fixed.

* Increment the ref counter to prevent an update operation (by put/2
* or erase/1) to delete this hash table.
*/
erts_atomic_inc_nob(&hash_table->refc);

This comment has been minimized.

@sverker

sverker Oct 24, 2018

Contributor

I don't think this hash_table->refc bump in get/0 and info/0 is not enough to keep the referred literals alive. If two or more updating operations are done before the trapping is done, the hash tables may be deallocated out of order and literals referred by this hash_table may be purged when a newer hash table is deallocated.

One solution could be to keep the hash tables in a linked list and make sure they are deallocated in order, older before newer.

This comment has been minimized.

@bjorng

bjorng Oct 26, 2018

Contributor

Fixed in the way that you suggested.

While at it, I also made sure that there will not be a memory leak if a process is killed while calling get/0 and info/0.

@bjorng bjorng force-pushed the bjorng:bjorn/erts/persistent_terms/OTP-14669 branch 2 times, most recently from e92d877 to d3f0571 Oct 25, 2018

Refactor releasing of literals
Introudce erts_queue_release_literals() to queue a literal area to be
released.

@bjorng bjorng force-pushed the bjorng:bjorn/erts/persistent_terms/OTP-14669 branch from 23ed43b to 27fd0b6 Oct 29, 2018

bjorng and others added some commits Oct 4, 2018

Add a persistent term storage
Persistent terms are useful for storing Erlang terms that are never
or infrequently updated. They have the following advantages:

* Constant time access. A persistent term is not copied when it is
  looked up. The constant factor is lower than for ETS, and no locks
  are taken when looking up a term.

* Persistent terms are not copied in garbage collections.

* There is only ever one copy of a persistent term (until it is
  deleted). That makes them useful for storing configuration data
  that needs to be easily accessible by all processes.

Persistent terms have the following drawbacks:

* Updates are expensive. The hash table holding the keys for the
  persistent terms are updated whenever a persistent term is added,
  updated or deleted.

* Updating or deleting a persistent term triggers a "global GC", which
  will schedule a heap scan of all processes to search the heap of all
  processes for the deleted term. If a process still holds a reference
  to the deleted term, the process will be garbage collected and the
  term copied to the heap of the process. This global GC can make the
  system less responsive for some time.

Three BIFs (implemented in C in the emulator) is the entire
interface to the persistent term functionality:

* put(Key, Value) to store a persistent term.

* get(Key) to look up a persistent term.

* erase(Key) to delete a persistent term.

There are also two additional BIFs to obtain information about
persistent terms:

* info() to return a map with information about persistent terms.

* get() to return a list of a {Key,Value} tuples for all persistent
  terms. (The values are not copied.)
Implement a tab for persistent terms in crashdump viewer
Co-authored-by: Siri Hansen <siri@erlang.org>

@bjorng bjorng force-pushed the bjorng:bjorn/erts/persistent_terms/OTP-14669 branch from d1a462b to 7489e81 Nov 6, 2018

@bjorng bjorng merged commit 7489e81 into erlang:maint Nov 6, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@bjorng bjorng deleted the bjorng:bjorn/erts/persistent_terms/OTP-14669 branch Nov 6, 2018

@archSeer archSeer referenced this pull request Jan 4, 2019

Open

ets bottle neck. #69

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment