WIP: Filterx non atomic refcount #925
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hold enough entries Since we only partially fill the hashtable for the dict, make sure we size the table properly, otherwise we end up resizing the dict at runtime. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…block The weakref array can only contain objects that were created by the current or subsequent filterx blocks. This means as we are exiting, we can free everything that is not needed anymore. Apart from the memory use, these weakrefs can also hurt performance: they may keep extra FilterXRefs around that point to mutable objects. When changing those, that will trigger a copy-on-write. By freeing the unused references, we can avoid the copy. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
This patch adds the management of a thread specific allocator along with weakrefs to be part of the evaluation context. The allocation API comes in a separate patch. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
This adds filterx_new_object(), filterx_malloc_object() and filterx_free_object() functions. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
bazsi
force-pushed
the
filterx-non-atomic-refcount
branch
from
bf84526 to
25420ee
Compare
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…ry() Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Instead of naming xrefs floating by convention, make it explicit with a flag and associated functions. xrefs can be floated using `filterx_ref_float()` and grounded using `filterx_ref_ground()`. Once an xref is floating, it can be converted into a grounded xref without doing a clone operation. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…ction Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…ject_add_object() The subject of all FilterXObject methods is an object anyway, so it does not add much value and is inconsistent. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
The new method should perform changes on @self and return a ref to self if it was successful. If in-place addition fails for some reason, it can return an alternative object. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
The FilterXRef object can absorb the returned valule by add_inplace(). If the value is the same object (e.g. inplace add was successful), we just return the same ref. If it was changed, then self->value is updated. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
… one Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…d goto This time, it is easier to just use an else block. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Instead of an ugly `add_mode` property, use a specific method. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
# Conflicts: # lib/filterx/filterx-globals.c
# Conflicts: # lib/filterx/filterx-expr.c
# Conflicts: # lib/filterx/object-string.c # lib/filterx/object-string.h # lib/str-utils.h
# Conflicts: # lib/filterx/object-string.c # lib/filterx/object-string.h
# Conflicts: # lib/filterx/expr-assign.c # lib/filterx/filterx-eval.h # lib/filterx/filterx-grammar.ym
…_slice() Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…t their use filterx_non_literal_from_expr_new() -> filterx_expr_wrapper_new(), as this is only used for generating errors in multiple compound expressions. filterx_non_literal_new() -> filterx_object_expr_new(), the point here is that we are using something other than FilterXLiteral, and previously this was solved using a FilterXLiteral embedded in expr wrapper. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
I intend to freeze literal->object automatically by FilterXLiteral, but doing so would make the object read-only. This is not going to work, as the unit tests themselves wrap writable objects into FilterXLiteral. Let's use the new FilterXObjectExpr instead, which does not freeze. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…entation file Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…ricted_context() These functions make it possible to compile/optimize FilterX code and load the cached version of JSON documents, but without using the globals in FilterXConfig. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
This patch introduces the freezing of mutable objects along with non-mutable ones, in order to allow sharing instances between threads, even without a non-atomic reference count. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
It is better to use the context than GlobalConfig to freeze, and also this allows us to simplify the call sites. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…terXObject Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
The "readonly" property of objects has been used to mark cached_json_file()
values, for two reasons:
1) all changes were rejected at the expr level
2) for read-only mutable objects, FilterXRef instances were not created as
we were looking up values (e.g. performance).
3) we did not want to allow changes to the underlying "shared" JSON object
The check in 1) is redundant, as the object layer is happy to decide if it
is read-only or not (e.g. strings will lack a set_subscript/setattr method
as they are immutable)
Avoiding the FilterXRef allocation in 2) is a good trick, however currently
anything we copy out of a readonly dict remains readonly, which is a bug
that needs to be fixed:
d = cached_json_file(...);
x = d.foo.bar;
# both d and x are are readonly.
With the latest changes/bugfixes in the FilterXRef machinery we are now able
to properly track cow even for shared dict instances, which we needed for
literal dicts/lists as well. This means that special handling of cached
json file values are not needed anymore.
This change drops readonly support for mutable objects.
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Literals need to be frozen, even if they are immutable, as the reference count is going to become a non-atomic value. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
This patch implements validation, so that in DEBUG builds we get a failed assertion when trying to ref/unref any objects that are allocated outside of a normal filterx evaluation. Most of the objects we work on are allocated while the filterx code is evaluated, however we have persisted objects too (hibernated or frozen), whose lifecycle is managed by the configuration (or cache_json_file()). These objects need to be hibernated or frozen. If a freeze call is missing, we would get ugly memory corruptions as FilterXObject->ref_cnt is not atomic. We basically detect "should-be-frozen" objects by marking allocations that are done within filterx_eval_begin_compile()/end_compile(). These objects either need to be frozen, or not used at all. The check if freeze was indeed called is done in filterx_object_ref/unref() calls. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
bazsi
force-pushed
the
filterx-non-atomic-refcount
branch
from
67d52db to
3c5b21c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Another attempt at removing the atomic ref counter from FilterXObjects for a cca 15% performance improvement (single thread), but which can also improve scalability to more CPUs).
It builds on all of my pending PRs as a whole (over 100 patches), so merging this would only happen once the base is smaller.
The core idea:
I am just opening this to get feedback, run the CI and prepare for discussions.