-
Notifications
You must be signed in to change notification settings - Fork 116
add blazingly fast instance_cache implementation for K2 #1683
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
base: master
Are you sure you want to change the base?
Changes from all commits
40aa5cc
3ff6647
ce9f1a1
0d128df
eb40a15
697be39
9afe36d
cf11ba5
c661e8b
de5c746
1967d1c
b0ad895
5682085
a76b998
ac45877
458d1ff
274b972
d4e6c2d
dbba99f
a77d491
ef9401a
7f12b1a
e77d731
81cee3f
011584a
85df866
ffcf9fb
12b4120
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,10 @@ | ||
| <?php | ||
|
|
||
| /** @kphp-extern-func-info cpp_template_call interruptible */ | ||
| /** @kphp-extern-func-info cpp_template_call */ | ||
| function instance_cache_fetch(string $type, string $key, bool $even_if_expired = false) ::: instance<^1>; | ||
|
|
||
| /** @kphp-extern-func-info interruptible */ | ||
| function instance_cache_store(string $key, object $value, int $ttl = 0) ::: bool; | ||
|
|
||
| /** @kphp-extern-func-info interruptible */ | ||
| function instance_cache_update_ttl(string $key, int $ttl = 0) ::: bool; | ||
|
|
||
| /** @kphp-extern-func-info interruptible */ | ||
| function instance_cache_delete(string $key) ::: bool; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| #include "compiler/code-gen/declarations.h" | ||
|
|
||
| #include "common/algorithms/compare.h" | ||
| #include "common/algorithms/hashes.h" | ||
|
|
||
| #include "compiler/code-gen/common.h" | ||
| #include "compiler/code-gen/const-globals-batched-mem.h" | ||
|
|
@@ -563,6 +564,7 @@ void ClassDeclaration::compile_inner_methods(CodeGenerator& W, ClassPtr klass) { | |
| compile_has_wakeup_flag(W, klass); | ||
| compile_get_class(W, klass); | ||
| compile_get_hash(W, klass); | ||
| compile_class_name_hash(W, klass); | ||
| compile_accept_visitor_methods(W, klass); | ||
| compile_msgpack_declarations(W, klass); | ||
| compile_virtual_builtin_functions(W, klass); | ||
|
|
@@ -734,6 +736,12 @@ void ClassDeclaration::compile_get_hash(CodeGenerator& W, ClassPtr klass) { | |
| compile_class_method(FunctionSignatureGenerator(W).set_const_this(), klass, "int get_hash()", klass->get_hash()); | ||
| } | ||
|
|
||
| void ClassDeclaration::compile_class_name_hash(CodeGenerator& W, ClassPtr klass) { | ||
| // hash of the class name, computed once at compile time -- same for every instance, | ||
| // unlike the virtual get_hash() it can be read without an instance at hand. | ||
| W << "constexpr static uint64_t CLASS_NAME_HASH{" << vk::murmur_hash<uint64_t>(klass->name.data(), klass->name.size()) << "ULL};" << NL << NL; | ||
| } | ||
|
|
||
| void ClassDeclaration::compile_accept_visitor(CodeGenerator& W, ClassPtr klass, const char* visitor_type) { | ||
| compile_class_method(FunctionSignatureGenerator(W), klass, fmt_format("void accept({} &visitor)", visitor_type), "generic_accept(visitor)"); | ||
| } | ||
|
|
@@ -889,8 +897,7 @@ void ClassDeclaration::compile_accept_json_visitor(CodeGenerator& W, ClassPtr kl | |
| } | ||
|
|
||
| void ClassDeclaration::compile_accept_visitor_methods(CodeGenerator& W, ClassPtr klass) { | ||
| bool need_generic_accept = | ||
| klass->need_to_array_debug_visitor || (klass->need_instance_cache_visitors && !G->is_output_mode_k2()) || (klass->need_instance_memory_estimate_visitor); | ||
| bool need_generic_accept = klass->need_to_array_debug_visitor || klass->need_instance_cache_visitors || klass->need_instance_memory_estimate_visitor; | ||
|
|
||
| if (!need_generic_accept && klass->json_encoders.empty()) { | ||
| return; | ||
|
|
@@ -922,6 +929,13 @@ void ClassDeclaration::compile_accept_visitor_methods(CodeGenerator& W, ClassPtr | |
| compile_accept_visitor(W, klass, "InstanceDeepDestroyVisitor"); | ||
| } | ||
|
|
||
| if (klass->need_instance_cache_visitors && G->is_output_mode_k2()) { | ||
| W << NL; | ||
| compile_accept_visitor(W, klass, "kphp::visitors::instance_deep_copy_visitor"); | ||
| W << NL; | ||
| compile_accept_visitor(W, klass, "kphp::visitors::instance_deep_estimate_size_visitor"); | ||
| } | ||
|
|
||
| compile_accept_json_visitor(W, klass); | ||
| } | ||
|
|
||
|
|
@@ -942,8 +956,13 @@ void ClassDeclaration::compile_virtual_builtin_functions(CodeGenerator& W, Class | |
|
|
||
| compile_class_method(FunctionSignatureGenerator(W).set_const_this(), klass, "size_t virtual_builtin_sizeof()", "sizeof(*this)"); | ||
|
|
||
| compile_class_method(FunctionSignatureGenerator(W).set_const_this(), klass, "size_t virtual_builtin_alignof()", "alignof(" + klass->src_name + ")"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this? |
||
|
|
||
| compile_class_method(FunctionSignatureGenerator(W).set_const_this(), klass, klass->src_name + "* virtual_builtin_clone()", | ||
| "new " + klass->src_name + "{*this}"); | ||
|
|
||
| compile_class_method(FunctionSignatureGenerator(W).set_const_this(), klass, klass->src_name + "* virtual_builtin_construct_at(void* ptr)", | ||
| "new (ptr) " + klass->src_name + "{*this}"); | ||
|
denisichh marked this conversation as resolved.
|
||
| } | ||
|
|
||
| void ClassDeclaration::compile_wakeup(CodeGenerator& W, ClassPtr klass) { | ||
|
|
@@ -1038,8 +1057,7 @@ void ClassDeclaration::compile_job_worker_shared_memory_piece_methods(CodeGenera | |
| } | ||
|
|
||
| void ClassMembersDefinition::compile(CodeGenerator& W) const { | ||
| bool need_generic_accept = | ||
| klass->need_to_array_debug_visitor || (klass->need_instance_cache_visitors && !G->is_output_mode_k2()) || (klass->need_instance_memory_estimate_visitor); | ||
| bool need_generic_accept = klass->need_to_array_debug_visitor || klass->need_instance_cache_visitors || klass->need_instance_memory_estimate_visitor; | ||
|
|
||
| if (!need_generic_accept && !klass->is_serializable && klass->json_encoders.empty()) { | ||
| return; | ||
|
|
@@ -1080,6 +1098,13 @@ void ClassMembersDefinition::compile(CodeGenerator& W) const { | |
| compile_generic_accept_instantiations(W, klass, "InstanceDeepDestroyVisitor"); | ||
| } | ||
|
|
||
| if (klass->need_instance_cache_visitors && G->is_output_mode_k2()) { | ||
| W << NL; | ||
| compile_generic_accept_instantiations(W, klass, "kphp::visitors::instance_deep_copy_visitor"); | ||
| W << NL; | ||
| compile_generic_accept_instantiations(W, klass, "kphp::visitors::instance_deep_estimate_size_visitor"); | ||
| } | ||
|
|
||
| W << NL; | ||
| compile_accept_json_visitor(W, klass); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| #pragma once | ||
|
|
||
| #include <cstddef> | ||
|
|
||
| #include "common/smart_ptrs/intrusive_ptr.h" | ||
| #include "common/wrappers/span.h" | ||
|
|
||
| #ifndef INCLUDED_FROM_KPHP_CORE | ||
| #error "this file must be included only from runtime-core.h" | ||
|
|
@@ -71,8 +74,17 @@ public: | |
|
|
||
| inline class_instance& operator=(const Optional<bool>& null) noexcept; | ||
| inline class_instance clone() const; | ||
| // copies the instance into externally provided memory (no allocation/ownership) | ||
| // memory must be aligned to alignof(T) and >= estimate_memory_usage() bytes | ||
| // caller must pin it with a special ExtraRefCnt (e.g. for_instance_cache), since the instance never frees it. | ||
| // Returns a null instance if memory is unfit. | ||
| inline class_instance clone_in(vk::span<std::byte> memory) const noexcept; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have an ability to return something like variant? |
||
| template<class... Args> | ||
| inline class_instance<T> alloc(Args&&... args) __attribute__((always_inline)); | ||
| // constructs an instance in externally provided memory (no allocation/ownership) | ||
| // leaves it null if memory is smaller than sizeof(T) or misaligned | ||
| template<class... Args> | ||
| inline class_instance<T> alloc(vk::span<std::byte> memory, Args&&... args) noexcept __attribute__((always_inline)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that would be better to rename into
|
||
| inline class_instance<T> empty_alloc() __attribute__((always_inline)); | ||
| inline void destroy() { | ||
| o.reset(); | ||
|
|
@@ -97,11 +109,26 @@ public: | |
| return o->virtual_builtin_sizeof(); | ||
| } | ||
|
|
||
| template<class S = T> | ||
| std::enable_if_t<!std::is_polymorphic<S>{}, size_t> alignment() const noexcept { | ||
| return alignof(T); | ||
| } | ||
|
|
||
| template<class S = T> | ||
| std::enable_if_t<std::is_polymorphic<S>{}, size_t> alignment() const noexcept { | ||
| return o->virtual_builtin_alignof(); | ||
| } | ||
|
|
||
| template<class S = T> | ||
| std::enable_if_t<!std::is_polymorphic<S>{}, class_instance> virtual_builtin_clone() const noexcept { | ||
| return clone(); | ||
| } | ||
|
|
||
| template<class S = T> | ||
| std::enable_if_t<!std::is_polymorphic<S>{}, class_instance> virtual_builtin_clone_in(vk::span<std::byte> memory) const noexcept { | ||
| return clone_in(memory); | ||
| } | ||
|
|
||
| template<class S = T> | ||
| std::enable_if_t<std::is_polymorphic<S>{}, class_instance> virtual_builtin_clone() const noexcept { | ||
| // TODO this is used only for job workers. Should we use this logic for other? | ||
|
|
@@ -113,6 +140,19 @@ public: | |
| return res; | ||
| } | ||
|
|
||
| template<class S = T> | ||
| std::enable_if_t<std::is_polymorphic<S>{}, class_instance> virtual_builtin_clone_in(vk::span<std::byte> memory) const noexcept { | ||
| class_instance res; | ||
| if (o) { | ||
| if (unlikely(memory.size() < o->virtual_builtin_sizeof() || reinterpret_cast<std::uintptr_t>(memory.data()) % o->virtual_builtin_alignof() != 0)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend to add nullptr check for |
||
| return res; | ||
| } | ||
| res.o = vk::intrusive_ptr<T>{o->virtual_builtin_construct_at(memory.data())}; | ||
| res.o->set_refcnt(1); | ||
| } | ||
| return res; | ||
| } | ||
|
|
||
| template<class S = T> | ||
| std::enable_if_t<!std::is_polymorphic<S>{}, void*> get_base_raw_ptr() const noexcept { | ||
| return get(); | ||
|
|
@@ -198,6 +238,8 @@ public: | |
| private: | ||
| class_instance<T> clone_impl(std::true_type /*is empty*/) const; | ||
| class_instance<T> clone_impl(std::false_type /*is empty*/) const; | ||
| class_instance<T> clone_in_impl(vk::span<std::byte> memory, std::true_type /*is empty*/) const noexcept; | ||
| class_instance<T> clone_in_impl(vk::span<std::byte> memory, std::false_type /*is empty*/) const noexcept; | ||
| }; | ||
|
|
||
| template<class T, class... Args> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that it would be better not to introduce a new field. We already have
get_hashinclass_instance, which can be helpful, but it has a specific corner case:class_instance<T>{}.get_hash() == nullptr, while class_instance<T>{}.alloc().get_hash() != nullptr. I think you have two possible options: