Skip to content

Commit 8ad75ce

Browse files
jwtownerjwtowner
committed
Switch attribute_frame_info representation to use std::vector instead of linked list
- Replaced `scope_fail` with `scope_exit` in various methods for improved exception safety during memory management. - Updated exception throwing to use `lug::invalid_argument` for better error reporting in `detail.hpp`. - Refactored `attribute_frame_info` to utilize a vector for descriptors, enhancing memory management and reducing complexity. - Adjusted `attribute_frame_handle` to use indices instead of pointers for better performance and clarity. - Improved `empty()` method to check the size of descriptors instead of relying on a unique pointer.
1 parent 43c445a commit 8ad75ce

File tree

2 files changed

+58
-75
lines changed

2 files changed

+58
-75
lines changed

‎include/lug/detail.hpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -641,13 +641,13 @@ class stack_allocator
641641
, large_object_threshold_{lobj_thresh}
642642
{
643643
if LUG_UNLIKELY(psize <= sizeof(page))
644-
lug::throw_exception<invalid_argument>("page size is too small");
644+
lug::throw_exception<lug::invalid_argument>("page size is too small");
645645
if LUG_UNLIKELY(palign < alignof(std::max_align_t))
646-
lug::throw_exception<invalid_argument>("page align is too small");
646+
lug::throw_exception<lug::invalid_argument>("page align is too small");
647647
if LUG_UNLIKELY((palign & (palign - 1)) != 0)
648-
lug::throw_exception<invalid_argument>("page align is not a power of two");
648+
lug::throw_exception<lug::invalid_argument>("page align is not a power of two");
649649
if LUG_UNLIKELY(lobj_thresh > (psize / 2))
650-
lug::throw_exception<invalid_argument>("large object threshold must be no greater than half the page size");
650+
lug::throw_exception<lug::invalid_argument>("large object threshold must be no greater than half the page size");
651651
auto const new_page{static_cast<page*>(::operator new[](page_size_, std::align_val_t{page_align_}))};
652652
auto const new_page_addr{reinterpret_cast<std::uintptr_t>(new_page)}; // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
653653
new_page->next = nullptr;
@@ -716,7 +716,7 @@ class stack_allocator
716716
return reinterpret_cast<void*>(new_addr); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast,performance-no-int-to-ptr)
717717
}
718718
auto const ptr{::operator new[](size, std::align_val_t{align})};
719-
scope_fail cleanup{[ptr, align]() noexcept { ::operator delete[](ptr, std::align_val_t{align}); }};
719+
scope_exit cleanup{[ptr, align]() noexcept { ::operator delete[](ptr, std::align_val_t{align}); }};
720720
large_objects_ = new large_object(large_objects_, ptr, size, align); // NOLINT(cppcoreguidelines-owning-memory)
721721
cleanup.release();
722722
return ptr;

‎include/lug/lug.hpp‎

Lines changed: 53 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -523,42 +523,45 @@ class attribute_frame_info : public std::enable_shared_from_this<attribute_frame
523523
{
524524
friend class attribute_frame_handle;
525525

526+
struct descriptor;
527+
528+
struct sentinel_operations
529+
{
530+
static void persist(std::byte* /*buffer*/, descriptor const* /*desc*/, descriptor const* /*last*/) {}
531+
static void restore(std::byte* /*buffer*/, descriptor const* /*desc*/) {}
532+
static void destroy(std::byte* /*buffer*/, descriptor const* /*desc*/) {}
533+
};
534+
526535
struct descriptor
527536
{
528537
using forward_operation_fn = void (*)(std::byte*, descriptor const*, descriptor const*);
529538
using reverse_operation_fn = void (*)(std::byte*, descriptor const*);
530-
std::unique_ptr<descriptor> prev;
531-
descriptor* next{nullptr};
532-
forward_operation_fn persist{nullptr};
533-
reverse_operation_fn restore{nullptr};
534-
reverse_operation_fn destroy{nullptr};
535539
std::size_t offset{0};
536540
void* target{nullptr};
537-
void const* type{nullptr};
538-
descriptor(forward_operation_fn pfn, reverse_operation_fn rfn, reverse_operation_fn dfn, std::size_t off, void* tar, void const* typ) noexcept
539-
: persist{pfn}, restore{rfn}, destroy{dfn}, offset{off}, target{tar}, type{typ} {}
541+
void const* type{&detail::type_info_tag_v<void>};
542+
forward_operation_fn persist{&sentinel_operations::persist};
543+
reverse_operation_fn restore{&sentinel_operations::restore};
544+
reverse_operation_fn destroy{&sentinel_operations::destroy};
545+
descriptor() noexcept = default;
546+
descriptor(std::size_t off, void* tar, void const* typ, forward_operation_fn pfn, reverse_operation_fn rfn, reverse_operation_fn dfn) noexcept
547+
: offset{off}, target{tar}, type{typ}, persist{pfn}, restore{rfn}, destroy{dfn} {}
540548
};
541549

542550
template <class T>
543-
struct descriptor_operations
551+
struct operations
544552
{
545553
static void persist(std::byte* buffer, descriptor const* desc, descriptor const* last)
546554
{
547555
if constexpr (std::is_nothrow_copy_constructible_v<T>) {
548556
::new(buffer + desc->offset) T{*static_cast<T const*>(desc->target)};
549557
} else {
550-
detail::scope_fail guard{[buffer, desc]() noexcept {
551-
if (descriptor const* prev = desc->prev.get(); prev)
552-
(*prev->destroy)(buffer, prev);
553-
}};
558+
detail::scope_exit guard{[buffer, desc]() noexcept { (*((desc - 1)->destroy))(buffer, desc - 1); }};
554559
::new(buffer + desc->offset) T{*static_cast<T const*>(desc->target)};
555560
guard.release();
556561
}
557562
if (desc == last)
558563
return;
559-
descriptor const* next = desc->next;
560-
if (next == nullptr)
561-
return;
564+
descriptor const* const next = desc + 1;
562565
LUG_MUSTTAIL return (*next->persist)(buffer, next, last); // NOLINT(readability-avoid-return-with-void-value)
563566
}
564567

@@ -570,46 +573,39 @@ class attribute_frame_info : public std::enable_shared_from_this<attribute_frame
570573
} else if constexpr (std::is_nothrow_copy_assignable_v<T>) {
571574
*static_cast<T*>(desc->target) = *from;
572575
} else if constexpr (std::is_move_assignable_v<T>) {
573-
detail::scope_fail guard{[buffer, desc]() noexcept { (*desc->destroy)(buffer, desc); }};
576+
detail::scope_exit guard{[buffer, desc]() noexcept { (*desc->destroy)(buffer, desc); }};
574577
*static_cast<T*>(desc->target) = static_cast<T&&>(*from);
575578
guard.release();
576579
} else {
577-
detail::scope_fail guard{[buffer, desc]() noexcept { (*desc->destroy)(buffer, desc); }};
580+
detail::scope_exit guard{[buffer, desc]() noexcept { (*desc->destroy)(buffer, desc); }};
578581
*static_cast<T*>(desc->target) = *from;
579582
guard.release();
580583
}
581584
std::destroy_at(from);
582-
descriptor const* prev = desc->prev.get();
583-
if (prev == nullptr)
584-
return;
585+
descriptor const* const prev = desc - 1;
585586
LUG_MUSTTAIL return (*prev->restore)(buffer, prev); // NOLINT(readability-avoid-return-with-void-value)
586587
}
587588

588589
static void destroy(std::byte* buffer, descriptor const* desc)
589590
{
590591
std::destroy_at(static_cast<T*>(static_cast<void*>(buffer + desc->offset))); // NOLINT(bugprone-casting-through-void)
591-
descriptor const* prev = desc->prev.get();
592-
if (prev == nullptr)
593-
return;
592+
descriptor const* const prev = desc - 1;
594593
LUG_MUSTTAIL return (*prev->destroy)(buffer, prev); // NOLINT(readability-avoid-return-with-void-value)
595594
}
596595
};
597596

598-
std::unique_ptr<descriptor> last_;
599-
descriptor* first_{nullptr};
597+
std::vector<descriptor> descriptors_{1};
600598
std::size_t align_bytes_{1};
601599
std::size_t size_bytes_{0};
602600

603601
[[nodiscard]] bool is_target_unique(void* target, void const* type) const
604602
{
605-
descriptor const* desc = last_.get();
606-
while (desc != nullptr) {
607-
if (desc->target == target) {
608-
if LUG_UNLIKELY(desc->type != type)
603+
for (auto const& desc : descriptors_) {
604+
if (desc.target == target) {
605+
if LUG_UNLIKELY(desc.type != type)
609606
lug::throw_exception<attribute_stack_error>();
610607
return false;
611608
}
612-
desc = desc->prev.get();
613609
}
614610
return true;
615611
}
@@ -621,7 +617,7 @@ class attribute_frame_info : public std::enable_shared_from_this<attribute_frame
621617
attribute_frame_info(attribute_frame_info&&) = delete;
622618
attribute_frame_info& operator=(attribute_frame_info const&) = delete;
623619
attribute_frame_info& operator=(attribute_frame_info&&) = delete;
624-
[[nodiscard]] bool empty() const noexcept { return !last_; }
620+
[[nodiscard]] bool empty() const noexcept { return descriptors_.size() <= 1; }
625621
[[nodiscard]] std::size_t alignment() const noexcept { return align_bytes_; }
626622
[[nodiscard]] std::size_t size_bytes() const noexcept { return size_bytes_; }
627623
[[nodiscard]] attribute_frame_handle handle() const;
@@ -632,17 +628,7 @@ class attribute_frame_info : public std::enable_shared_from_this<attribute_frame
632628
using U = std::remove_cv_t<T>;
633629
if (is_target_unique(target, &detail::type_info_tag_v<U>)) {
634630
std::size_t const offset{(size_bytes_ + (alignof(U) - 1)) & ~(alignof(U) - 1)};
635-
auto desc{std::make_unique<descriptor>(
636-
&descriptor_operations<U>::persist,
637-
&descriptor_operations<U>::restore,
638-
&descriptor_operations<U>::destroy,
639-
offset, target, &detail::type_info_tag_v<U>)};
640-
desc->prev = std::move(last_);
641-
if (desc->prev != nullptr)
642-
desc->prev->next = desc.get();
643-
if (first_ == nullptr)
644-
first_ = desc.get();
645-
last_ = std::move(desc);
631+
descriptors_.emplace_back(offset, target, &detail::type_info_tag_v<U>, &operations<U>::persist, &operations<U>::restore, &operations<U>::destroy);
646632
align_bytes_ = (std::max)(align_bytes_, alignof(U));
647633
size_bytes_ = offset + sizeof(U);
648634
}
@@ -653,33 +639,31 @@ class attribute_frame_handle
653639
{
654640
friend class attribute_frame_info;
655641
std::shared_ptr<attribute_frame_info const> info_;
656-
attribute_frame_info::descriptor const* first_{nullptr};
657-
attribute_frame_info::descriptor const* last_{nullptr};
658-
attribute_frame_handle(
659-
std::shared_ptr<attribute_frame_info const> info,
660-
attribute_frame_info::descriptor const* first,
661-
attribute_frame_info::descriptor const* last) noexcept
662-
: info_{std::move(info)}, first_{first}, last_{last} {}
642+
std::size_t head_{0};
643+
std::size_t tail_{0};
644+
attribute_frame_handle(std::shared_ptr<attribute_frame_info const> info, std::size_t first, std::size_t last) noexcept : info_{std::move(info)}, head_{first}, tail_{last} {}
645+
[[nodiscard]] LUG_ALWAYS_INLINE attribute_frame_info::descriptor const* head() const noexcept { return info_->descriptors_.data() + head_; }
646+
[[nodiscard]] LUG_ALWAYS_INLINE attribute_frame_info::descriptor const* tail() const noexcept { return info_->descriptors_.data() + tail_; }
663647
public:
664648
constexpr attribute_frame_handle() noexcept = default;
665649
attribute_frame_handle(attribute_frame_handle const&) noexcept = default;
666650
attribute_frame_handle(attribute_frame_handle&&) noexcept = default;
667651
attribute_frame_handle& operator=(attribute_frame_handle const&) noexcept = default;
668652
attribute_frame_handle& operator=(attribute_frame_handle&&) noexcept = default;
669653
~attribute_frame_handle() noexcept = default;
670-
[[nodiscard]] bool empty() const noexcept { return info_->empty(); }
671-
[[nodiscard]] std::size_t alignment() const noexcept { return info_->alignment(); }
672-
[[nodiscard]] std::size_t size_bytes() const noexcept { return info_->size_bytes(); }
673-
LUG_NONNULL(2) void persist(std::byte* buffer) const { (*first_->persist)(buffer, first_, last_); }
674-
LUG_NONNULL(2) void restore(std::byte* buffer) const { (*last_->restore)(buffer, last_); }
675-
LUG_NONNULL(2) void destroy(std::byte* buffer) const noexcept { (*last_->destroy)(buffer, last_); }
676-
[[nodiscard]] bool operator==(attribute_frame_handle const& other) const noexcept { return info_ == other.info_ && first_ == other.first_ && last_ == other.last_; }
677-
[[nodiscard]] bool operator!=(attribute_frame_handle const& other) const noexcept { return !(*this == other); }
654+
[[nodiscard]] LUG_ALWAYS_INLINE bool empty() const noexcept { return info_->empty(); }
655+
[[nodiscard]] LUG_ALWAYS_INLINE std::size_t alignment() const noexcept { return info_->alignment(); }
656+
[[nodiscard]] LUG_ALWAYS_INLINE std::size_t size_bytes() const noexcept { return info_->size_bytes(); }
657+
LUG_ALWAYS_INLINE LUG_NONNULL(2) void persist(std::byte* buffer) const { (*(head()->persist))(buffer, head(), tail()); }
658+
LUG_ALWAYS_INLINE LUG_NONNULL(2) void restore(std::byte* buffer) const { (*(tail()->restore))(buffer, tail()); }
659+
LUG_ALWAYS_INLINE LUG_NONNULL(2) void destroy(std::byte* buffer) const noexcept { (*(tail()->destroy))(buffer, tail()); }
660+
[[nodiscard]] LUG_ALWAYS_INLINE bool operator==(attribute_frame_handle const& other) const noexcept { return info_ == other.info_ && head_ == other.head_ && tail_ == other.tail_; }
661+
[[nodiscard]] LUG_ALWAYS_INLINE bool operator!=(attribute_frame_handle const& other) const noexcept { return !(*this == other); }
678662
};
679663

680664
[[nodiscard]] inline attribute_frame_handle attribute_frame_info::handle() const
681665
{
682-
return attribute_frame_handle{shared_from_this(), first_, last_.get()};
666+
return attribute_frame_handle{shared_from_this(), ((descriptors_.size() > 1U) ? 1U : 0U), (descriptors_.size() - 1U)};
683667
}
684668

685669
struct program
@@ -964,15 +948,14 @@ class environment
964948
}
965949

966950
template <class FrameOp, class = std::enable_if_t<std::is_invocable_v<FrameOp, attribute_frame_handle const&, std::byte*>>>
967-
void pop_attribute_frame_instance(FrameOp const& frame_op)
968-
noexcept(std::is_nothrow_invocable_v<FrameOp, attribute_frame_handle const&, std::byte*>)
951+
void pop_attribute_frame_instance(FrameOp const& frame_op) noexcept(std::is_nothrow_invocable_v<FrameOp, attribute_frame_handle const&, std::byte*>)
969952
{
970953
attribute_frame_instance* const instance{attribute_frame_stack_};
971954
attribute_frame_instance* const next_instance{instance->next};
972955
std::byte* const buffer{instance->buffer};
973956
attribute_frame_handle const frame{std::move(instance->frame)};
974957
std::destroy_at(instance);
975-
detail::scope_exit release_memory{[this, &frame, buffer, instance, next_instance]() noexcept {
958+
detail::scope_exit const release_memory{[this, &frame, buffer, instance, next_instance]() noexcept {
976959
if (frame.size_bytes() >= attribute_frame_allocator_.large_object_threshold())
977960
attribute_frame_allocator_.rewind(buffer, frame.size_bytes(), frame.alignment());
978961
attribute_frame_allocator_.rewind(instance, sizeof(attribute_frame_instance), alignof(attribute_frame_instance));
@@ -1003,11 +986,11 @@ class environment
1003986
[[nodiscard]] attribute_collection tail_attribute_collection(std::size_t element_count = 1);
1004987
[[nodiscard]] bool has_condition(std::string_view name) const noexcept { return (conditions_.count(name) > 0); }
1005988
bool set_condition(std::string_view name, bool value) { return value ? (!conditions_.emplace(name).second) : (conditions_.erase(name) > 0); }
1006-
void clear_conditions() { conditions_.clear(); }
989+
void clear_conditions() noexcept { conditions_.clear(); }
1007990
[[nodiscard]] bool has_symbol(std::string_view name) const noexcept { return (symbols_.count(name) > 0); }
1008991
[[nodiscard]] std::vector<std::string> const& get_symbols(std::string_view name) const noexcept { auto it = symbols_.find(name); if (it == symbols_.end()) return empty_symbols_; return it->second; }
1009992
void add_symbol(std::string_view name, std::string value) { symbols_[name].emplace_back(std::move(value)); }
1010-
void clear_symbols(std::string_view name) { symbols_.erase(name); }
993+
void clear_symbols(std::string_view name) noexcept { symbols_.erase(name); }
1011994
[[nodiscard]] std::string_view match() const noexcept { return match_; }
1012995
[[nodiscard]] std::string_view subject() const noexcept { return subject_; }
1013996
[[nodiscard]] syntax_position position_begin(syntax const& stx) { return position_at(stx.index()); }
@@ -1018,7 +1001,7 @@ class environment
10181001
[[nodiscard]] std::pair<syntax_position, syntax_position> position_range(syntax_range const& range) { return {position_begin(range), position_end(range)}; }
10191002
[[nodiscard]] std::size_t call_depth() const noexcept { return call_depth_; }
10201003
[[nodiscard]] std::size_t prune_depth() const noexcept { return prune_depth_; }
1021-
void escape() { prune_depth_ = call_depth_; }
1004+
void escape() noexcept { prune_depth_ = call_depth_; }
10221005

10231006
[[nodiscard]] syntax_position position_at(std::size_t index)
10241007
{
@@ -1061,11 +1044,11 @@ class environment
10611044
void push_attribute_frame(attribute_frame_handle const& frame)
10621045
{
10631046
void* const instance_storage{attribute_frame_allocator_.allocate(sizeof(attribute_frame_instance), alignof(attribute_frame_instance))};
1064-
detail::scope_fail instance_cleanup{[this, instance_storage]() noexcept { attribute_frame_allocator_.rewind(instance_storage, sizeof(attribute_frame_instance), alignof(attribute_frame_instance)); }};
1047+
detail::scope_exit instance_cleanup{[this, instance_storage]() noexcept { attribute_frame_allocator_.rewind(instance_storage, sizeof(attribute_frame_instance), alignof(attribute_frame_instance)); }};
10651048
std::byte* const buffer{static_cast<std::byte*>(attribute_frame_allocator_.allocate(frame.size_bytes(), frame.alignment()))};
1066-
detail::scope_fail buffer_cleanup{[this, &frame, buffer]() noexcept { attribute_frame_allocator_.rewind(buffer, frame.size_bytes(), frame.alignment()); }};
1049+
detail::scope_exit buffer_cleanup{[this, &frame, buffer]() noexcept { attribute_frame_allocator_.rewind(buffer, frame.size_bytes(), frame.alignment()); }};
10671050
frame.persist(buffer);
1068-
detail::scope_fail frame_cleanup{[&frame, buffer]() noexcept { frame.destroy(buffer); }};
1051+
detail::scope_exit frame_cleanup{[&frame, buffer]() noexcept { frame.destroy(buffer); }};
10691052
attribute_frame_stack_ = ::new(instance_storage) attribute_frame_instance{attribute_frame_stack_, buffer, frame}; // NOLINT(cppcoreguidelines-owning-memory)
10701053
frame_cleanup.release();
10711054
buffer_cleanup.release();
@@ -1128,8 +1111,8 @@ class attribute_collection
11281111
~attribute_collection() { if (first_initial_ < last_initial_) envr_->attribute_result_stack_.resize(first_initial_); }
11291112
[[nodiscard]] bool empty() const noexcept { return first_ >= last_; }
11301113
[[nodiscard]] std::size_t size() const noexcept { return (first_ < last_) ? (last_ - first_) : 0; }
1131-
void consume_front(std::size_t n) { first_ += n; }
1132-
void consume_back(std::size_t n) { last_ -= (std::min)(n, last_); }
1114+
void consume_front(std::size_t n) noexcept { first_ += n; }
1115+
void consume_back(std::size_t n) noexcept { last_ -= (std::min)(n, last_); }
11331116
template <class T, std::size_t I> [[nodiscard]] T read_front() { return detail::move_only_any_cast<T>(std::move(envr_->attribute_result_stack_[first_ + I])); }
11341117
template <class T, std::size_t I, std::size_t N> [[nodiscard]] T read_back() { return detail::move_only_any_cast<T>(std::move(envr_->attribute_result_stack_[last_ - N + I])); }
11351118
attribute_collection(attribute_collection const&) = delete;

0 commit comments

Comments
 (0)