diff --git a/.github/workflows/install_grpc.sh b/.github/workflows/install_grpc.sh index f8b01e57a..36b31bcae 100755 --- a/.github/workflows/install_grpc.sh +++ b/.github/workflows/install_grpc.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -git clone --branch v1.44.0 https://github.com/grpc/grpc +git clone --branch v1.54.3 https://github.com/grpc/grpc cd grpc git submodule update --init mkdir -p cmake/build diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e00a87d8d..5ad7256f0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -47,15 +47,10 @@ jobs: build: strategy: matrix: - os: [ubuntu-22.04, macos-13] + os: [ubuntu-22.04, ubuntu-24.04, macos-13] include: - - os: ubuntu-24.04 - cc: gcc-12 - cxx: g++-12 - os: ubuntu-latest sanitizer: ASAN - cc: gcc-10 - cxx: g++-10 - os: ubuntu-latest sanitizer: TSAN cc: gcc-12 @@ -119,8 +114,6 @@ jobs: include: - os: ubuntu-latest sanitizer: ASAN - cc: gcc-10 - cxx: g++-10 test: 'C++' - os: ubuntu-latest sanitizer: ASAN diff --git a/clang-tidy/.clang-tidy b/clang-tidy/.clang-tidy deleted file mode 100644 index 063df74f1..000000000 --- a/clang-tidy/.clang-tidy +++ /dev/null @@ -1,31 +0,0 @@ -Checks: 'clang-diagnostic-*, - clang-analyzer-*, - performance-*, - bugprone-*, - -bugprone-exception-escape, - -bugprone-branch-clone, - -bugprone-easily-swappable-parameters, - -bugprone-macro-parentheses, - -bugprone-signed-char-misuse, - -bugprone-narrowing-conversions, - -bugprone-reserved-identifier, - -bugprone-implicit-widening-of-multiplication-result, - -bugprone-assignment-in-if-condition, - -bugprone-parent-virtual-call, - -bugprone-integer-division, - -bugprone-unhandled-self-assignment, - -bugprone-inc-dec-in-conditions, - -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling, - -performance-no-int-to-ptr, - -performance-enum-size, - -performance-avoid-endl' -# clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling - too many unnecessary warning in vendored code -# performance-no-int-to-ptr - consider how to fix this -# bugprone-macro-parentheses - consider fixing -WarningsAsErrors: '*' -HeaderFilterRegex: '.*(?= 4.0.0 are given under - # the top level key 'Diagnostics' in the output yaml files - mergekey = "Diagnostics" - merged=[] - for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')): - content = yaml.safe_load(open(replacefile, 'r')) - if not content: - continue # Skip empty files. - merged.extend(content.get(mergekey, [])) - - if merged: - # MainSourceFile: The key is required by the definition inside - # include/clang/Tooling/ReplacementsYaml.h, but the value - # is actually never used inside clang-apply-replacements, - # so we set it to '' here. - output = {'MainSourceFile': '', mergekey: merged} - with open(mergefile, 'w') as out: - yaml.safe_dump(output, out) - else: - # Empty the file: - open(mergefile, 'w').close() - - -def find_binary(arg, name, build_path): - """Get the path for a binary or exit""" - if arg: - if shutil.which(arg): - return arg - else: - raise SystemExit( - "error: passed binary '{}' was not found or is not executable" - .format(arg)) - - built_path = os.path.join(build_path, "bin", name) - binary = shutil.which(name) or shutil.which(built_path) - if binary: - return binary - else: - raise SystemExit( - "error: failed to find {} in $PATH or at {}" - .format(name, built_path)) - - -def apply_fixes(args, clang_apply_replacements_binary, tmpdir): - """Calls clang-apply-fixes on a given directory.""" - invocation = [clang_apply_replacements_binary] - invocation.append('-ignore-insert-conflict') - if args.format: - invocation.append('-format') - if args.style: - invocation.append('-style=' + args.style) - invocation.append(tmpdir) - subprocess.call(invocation) - - -def run_tidy(args, clang_tidy_binary, tmpdir, build_path, queue, lock, - failed_files): - """Takes filenames out of queue and runs clang-tidy on them.""" - while True: - name = queue.get() - invocation = get_tidy_invocation(name, clang_tidy_binary, args.checks, - tmpdir, build_path, args.header_filter, - args.allow_enabling_alpha_checkers, - args.extra_arg, args.extra_arg_before, - args.quiet, args.config_file, args.config, - args.line_filter, args.use_color, - args.plugins) - - proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - output, err = proc.communicate() - if proc.returncode != 0: - if proc.returncode < 0: - msg = "%s: terminated by signal %d\n" % (name, -proc.returncode) - err += msg.encode('utf-8') - failed_files.append(name) - with lock: - sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8')) - if len(err) > 0: - sys.stdout.flush() - sys.stderr.write(err.decode('utf-8')) - queue.task_done() - - -def main(): - parser = argparse.ArgumentParser(description='Runs clang-tidy over all files ' - 'in a compilation database. Requires ' - 'clang-tidy and clang-apply-replacements in ' - '$PATH or in your build directory.') - parser.add_argument('-allow-enabling-alpha-checkers', - action='store_true', help='allow alpha checkers from ' - 'clang-analyzer.') - parser.add_argument('-clang-tidy-binary', metavar='PATH', - default='clang-tidy-18', - help='path to clang-tidy binary') - parser.add_argument('-clang-apply-replacements-binary', metavar='PATH', - default='clang-apply-replacements-18', - help='path to clang-apply-replacements binary') - parser.add_argument('-checks', default=None, - help='checks filter, when not specified, use clang-tidy ' - 'default') - config_group = parser.add_mutually_exclusive_group() - config_group.add_argument('-config', default=None, - help='Specifies a configuration in YAML/JSON format: ' - ' -config="{Checks: \'*\', ' - ' CheckOptions: {x: y}}" ' - 'When the value is empty, clang-tidy will ' - 'attempt to find a file named .clang-tidy for ' - 'each source file in its parent directories.') - config_group.add_argument('-config-file', default=None, - help='Specify the path of .clang-tidy or custom config ' - 'file: e.g. -config-file=/some/path/myTidyConfigFile. ' - 'This option internally works exactly the same way as ' - '-config option after reading specified config file. ' - 'Use either -config-file or -config, not both.') - parser.add_argument('-header-filter', default=None, - help='regular expression matching the names of the ' - 'headers to output diagnostics from. Diagnostics from ' - 'the main file of each translation unit are always ' - 'displayed.') - parser.add_argument('-line-filter', default=None, - help='List of files with line ranges to filter the' - 'warnings.') - if yaml: - parser.add_argument('-export-fixes', metavar='filename', dest='export_fixes', - help='Create a yaml file to store suggested fixes in, ' - 'which can be applied with clang-apply-replacements.') - parser.add_argument('-j', type=int, default=0, - help='number of tidy instances to be run in parallel.') - parser.add_argument('files', nargs='*', default=['.*'], - help='files to be processed (regex on path)') - parser.add_argument('-fix', action='store_true', help='apply fix-its') - parser.add_argument('-format', action='store_true', help='Reformat code ' - 'after applying fixes') - parser.add_argument('-style', default='file', help='The style of reformat ' - 'code after applying fixes') - parser.add_argument('-use-color', type=strtobool, nargs='?', const=True, - help='Use colors in diagnostics, overriding clang-tidy\'s' - ' default behavior. This option overrides the \'UseColor' - '\' option in .clang-tidy file, if any.') - parser.add_argument('-p', dest='build_path', - help='Path used to read a compile command database.') - parser.add_argument('-extra-arg', dest='extra_arg', - action='append', default=[], - help='Additional argument to append to the compiler ' - 'command line.') - parser.add_argument('-extra-arg-before', dest='extra_arg_before', - action='append', default=[], - help='Additional argument to prepend to the compiler ' - 'command line.') - parser.add_argument('-ignore', default=DEFAULT_CLANG_TIDY_IGNORE, - help='File path to clang-tidy-ignore') - parser.add_argument('-quiet', action='store_true', - help='Run clang-tidy in quiet mode') - parser.add_argument('-load', dest='plugins', - action='append', default=[], - help='Load the specified plugin in clang-tidy.') - args = parser.parse_args() - - db_path = 'compile_commands.json' - - if args.build_path is not None: - build_path = args.build_path - else: - # Find our database - build_path = find_compilation_database(db_path) - - clang_tidy_binary = find_binary(args.clang_tidy_binary, "clang-tidy", - build_path) - - tmpdir = None - if args.fix or (yaml and args.export_fixes): - clang_apply_replacements_binary = find_binary( - args.clang_apply_replacements_binary, "clang-apply-replacements", - build_path) - tmpdir = tempfile.mkdtemp() - - try: - invocation = get_tidy_invocation("", clang_tidy_binary, args.checks, - None, build_path, args.header_filter, - args.allow_enabling_alpha_checkers, - args.extra_arg, args.extra_arg_before, - args.quiet, args.config_file, args.config, - args.line_filter, args.use_color, - args.plugins) - invocation.append('-list-checks') - invocation.append('-') - if args.quiet: - # Even with -quiet we still want to check if we can call clang-tidy. - with open(os.devnull, 'w') as dev_null: - subprocess.check_call(invocation, stdout=dev_null) - else: - subprocess.check_call(invocation) - except: - print("Unable to run clang-tidy.", file=sys.stderr) - sys.exit(1) - - # Load the database and extract all files. - database = json.load(open(os.path.join(build_path, db_path))) - files = set([make_absolute(entry['file'], entry['directory']) - for entry in database]) - files, excluded = filter_files(args.ignore, files) - if excluded: - print("Excluding the following files:\n" + "\n".join(excluded) + "\n") - - max_task = args.j - if max_task == 0: - max_task = multiprocessing.cpu_count() - - # Build up a big regexy filter from all command line arguments. - file_name_re = re.compile('|'.join(args.files)) - - return_code = 0 - try: - # Spin up a bunch of tidy-launching threads. - task_queue = queue.Queue(max_task) - # List of files with a non-zero return code. - failed_files = [] - lock = threading.Lock() - for _ in range(max_task): - t = threading.Thread(target=run_tidy, - args=(args, clang_tidy_binary, tmpdir, build_path, - task_queue, lock, failed_files)) - t.daemon = True - t.start() - - # Fill the queue with files. - for name in files: - if file_name_re.search(name): - task_queue.put(name) - - # Wait for all threads to be done. - task_queue.join() - if len(failed_files): - return_code = 1 - - except KeyboardInterrupt: - # This is a sad hack. Unfortunately subprocess goes - # bonkers with ctrl-c and we start forking merrily. - print('\nCtrl-C detected, goodbye.') - if tmpdir: - shutil.rmtree(tmpdir) - os.kill(0, 9) - - if yaml and args.export_fixes: - print('Writing fixes to ' + args.export_fixes + ' ...') - try: - merge_replacement_files(tmpdir, args.export_fixes) - except: - print('Error exporting fixes.\n', file=sys.stderr) - traceback.print_exc() - return_code=1 - - if args.fix: - print('Applying fixes ...') - try: - apply_fixes(args, clang_apply_replacements_binary, tmpdir) - except: - print('Error applying fixes.\n', file=sys.stderr) - traceback.print_exc() - return_code = 1 - - if tmpdir: - shutil.rmtree(tmpdir) - sys.exit(return_code) - - -if __name__ == '__main__': - main() diff --git a/cpp_src/cmd/reindexer_server/test/test_storage_compatibility.sh b/cpp_src/cmd/reindexer_server/test/test_storage_compatibility.sh new file mode 100755 index 000000000..873181e20 --- /dev/null +++ b/cpp_src/cmd/reindexer_server/test/test_storage_compatibility.sh @@ -0,0 +1,197 @@ +#!/bin/bash +# Task: https://github.com/restream/reindexer/-/issues/1188 +set -e + +function KillAndRemoveServer { + local pid=$1 + kill $pid + wait $pid + yum remove -y 'reindexer*' > /dev/null +} + +function WaitForDB { + # wait until DB is loaded + set +e # disable "exit on error" so the script won't stop when DB's not loaded yet + is_connected=$(reindexer_tool --dsn $ADDRESS --command '\databases list'); + while [[ $is_connected != "test" ]] + do + sleep 2 + is_connected=$(reindexer_tool --dsn $ADDRESS --command '\databases list'); + done + set -e +} + +function CompareNamespacesLists { + local ns_list_actual=$1 + local ns_list_expected=$2 + local pid=$3 + + diff=$(echo ${ns_list_actual[@]} ${ns_list_expected[@]} | tr ' ' '\n' | sort | uniq -u) # compare in any order + if [ "$diff" == "" ]; then + echo "## PASS: namespaces list not changed" + else + echo "##### FAIL: namespaces list was changed" + echo "expected: $ns_list_expected" + echo "actual: $ns_list_actual" + KillAndRemoveServer $pid; + exit 1 + fi +} + +function CompareMemstats { + local actual=$1 + local expected=$2 + local pid=$3 + diff=$(echo ${actual[@]} ${expected[@]} | tr ' ' '\n' | sed 's/\(.*\),$/\1/' | sort | uniq -u) # compare in any order + if [ "$diff" == "" ]; then + echo "## PASS: memstats not changed" + else + echo "##### FAIL: memstats was changed" + echo "expected: $expected" + echo "actual: $actual" + KillAndRemoveServer $pid; + exit 1 + fi +} + + +RX_SERVER_CURRENT_VERSION_RPM="$(basename build/reindexer-*server*.rpm)" +VERSION_FROM_RPM=$(echo "$RX_SERVER_CURRENT_VERSION_RPM" | grep -o '.*server-..') +VERSION=$(echo ${VERSION_FROM_RPM: -2:1}) # one-digit version + +echo "## choose latest release rpm file" +if [ $VERSION == 3 ]; then + LATEST_RELEASE=$(python3 cpp_src/cmd/reindexer_server/test/get_last_rx_version.py -v 3) + namespaces_list_expected=$'purchase_options_ext_dict\nchild_account_recommendations\n#config\n#activitystats\nradio_channels\ncollections\n#namespaces\nwp_imports_tasks\nepg_genres\nrecom_media_items_personal\nrecom_epg_archive_default\n#perfstats\nrecom_epg_live_default\nmedia_view_templates\nasset_video_servers\nwp_tasks_schedule\nadmin_roles\n#clientsstats\nrecom_epg_archive_personal\nrecom_media_items_similars\nmenu_items\naccount_recommendations\nkaraoke_items\nmedia_items\nbanners\n#queriesperfstats\nrecom_media_items_default\nrecom_epg_live_personal\nservices\n#memstats\nchannels\nmedia_item_recommendations\nwp_tasks_tasks\nepg' +elif [ $VERSION == 4 ]; then + LATEST_RELEASE=$(python3 cpp_src/cmd/reindexer_server/test/get_last_rx_version.py -v 4) + # replicationstats ns added for v4 + namespaces_list_expected=$'purchase_options_ext_dict\nchild_account_recommendations\n#config\n#activitystats\n#replicationstats\nradio_channels\ncollections\n#namespaces\nwp_imports_tasks\nepg_genres\nrecom_media_items_personal\nrecom_epg_archive_default\n#perfstats\nrecom_epg_live_default\nmedia_view_templates\nasset_video_servers\nwp_tasks_schedule\nadmin_roles\n#clientsstats\nrecom_epg_archive_personal\nrecom_media_items_similars\nmenu_items\naccount_recommendations\nkaraoke_items\nmedia_items\nbanners\n#queriesperfstats\nrecom_media_items_default\nrecom_epg_live_personal\nservices\n#memstats\nchannels\nmedia_item_recommendations\nwp_tasks_tasks\nepg' +else + echo "Unknown version" + exit 1 +fi + +echo "## downloading latest release rpm file: $LATEST_RELEASE" +curl "http://repo.itv.restr.im/itv-api-ng/7/x86_64/$LATEST_RELEASE" --output $LATEST_RELEASE; +echo "## downloading example DB" +curl "https://github.com/restream/reindexer_testdata/-/raw/main/dump_demo.zip" --output dump_demo.zip; +unzip -o dump_demo.zip # unzips into demo_test.rxdump; + +ADDRESS="cproto://127.0.0.1:6534/" +DB_NAME="test" + +memstats_expected=$'[ +{"name":"account_recommendations","replication":{"data_hash":6833710705,"data_count":1}}, +{"name":"admin_roles","replication":{"data_hash":1896088071,"data_count":2}}, +{"name":"asset_video_servers","replication":{"data_hash":7404222244,"data_count":97}}, +{"name":"banners","replication":{"data_hash":0,"data_count":0}}, +{"name":"channels","replication":{"data_hash":457292509431319,"data_count":3941}}, +{"name":"child_account_recommendations","replication":{"data_hash":6252344969,"data_count":1}}, +{"name":"collections","replication":{"data_hash":0,"data_count":0}}, +{"name":"epg","replication":{"data_hash":-7049751653258,"data_count":1623116}}, +{"name":"epg_genres","replication":{"data_hash":8373644068,"data_count":1315}}, +{"name":"karaoke_items","replication":{"data_hash":5858155773472,"data_count":4500}}, +{"name":"media_item_recommendations","replication":{"data_hash":-6520334670,"data_count":35886}}, +{"name":"media_items","replication":{"data_hash":-1824301168479972392,"data_count":65448}}, +{"name":"media_view_templates","replication":{"data_hash":0,"data_count":0}}, +{"name":"menu_items","replication":{"data_hash":0,"data_count":0}}, +{"name":"purchase_options_ext_dict","replication":{"data_hash":24651210926,"data_count":3}}, +{"name":"radio_channels","replication":{"data_hash":37734732881,"data_count":28}}, +{"name":"recom_epg_archive_default","replication":{"data_hash":0,"data_count":0}}, +{"name":"recom_epg_archive_personal","replication":{"data_hash":0,"data_count":0}}, +{"name":"recom_epg_live_default","replication":{"data_hash":0,"data_count":0}}, +{"name":"recom_epg_live_personal","replication":{"data_hash":0,"data_count":0}}, +{"name":"recom_media_items_default","replication":{"data_hash":8288213744,"data_count":3}}, +{"name":"recom_media_items_personal","replication":{"data_hash":0,"data_count":0}}, +{"name":"recom_media_items_similars","replication":{"data_hash":-672103903,"data_count":33538}}, +{"name":"services","replication":{"data_hash":0,"data_count":0}}, +{"name":"wp_imports_tasks","replication":{"data_hash":777859741066,"data_count":1145}}, +{"name":"wp_tasks_schedule","replication":{"data_hash":12595790956,"data_count":4}}, +{"name":"wp_tasks_tasks","replication":{"data_hash":28692716680,"data_count":281}} +] +Returned 27 rows' + +echo "##### Forward compatibility test #####" + +DB_PATH=$(pwd)"/rx_db" + +echo "Database: "$DB_PATH + +echo "## installing latest release: $LATEST_RELEASE" +yum install -y $LATEST_RELEASE > /dev/null; +# run RX server with disabled logging +reindexer_server -l warning --httplog=none --rpclog=none --db $DB_PATH & +server_pid=$! +sleep 2; + +reindexer_tool --dsn $ADDRESS$DB_NAME -f demo_test.rxdump --createdb; +sleep 1; + +namespaces_1=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list'); +echo $namespaces_1; +CompareNamespacesLists "${namespaces_1[@]}" "${namespaces_list_expected[@]}" $server_pid; + +memstats_1=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select name, replication.data_hash, replication.data_count from #memstats order by name'); +CompareMemstats "${memstats_1[@]}" "${memstats_expected[@]}" $server_pid; + +KillAndRemoveServer $server_pid; + +echo "## installing current version: $RX_SERVER_CURRENT_VERSION_RPM" +yum install -y build/*.rpm > /dev/null; +reindexer_server -l0 --corelog=none --httplog=none --rpclog=none --db $DB_PATH & +server_pid=$! +sleep 2; + +WaitForDB + +namespaces_2=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list'); +echo $namespaces_2; +CompareNamespacesLists "${namespaces_2[@]}" "${namespaces_1[@]}" $server_pid; + + +memstats_2=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select name, replication.data_hash, replication.data_count from #memstats order by name'); +CompareMemstats "${memstats_2[@]}" "${memstats_1[@]}" $server_pid; + +KillAndRemoveServer $server_pid; +rm -rf $DB_PATH; +sleep 1; + +echo "##### Backward compatibility test #####" + +echo "## installing current version: $RX_SERVER_CURRENT_VERSION_RPM" +yum install -y build/*.rpm > /dev/null; +reindexer_server -l warning --httplog=none --rpclog=none --db $DB_PATH & +server_pid=$! +sleep 2; + +reindexer_tool --dsn $ADDRESS$DB_NAME -f demo_test.rxdump --createdb; +sleep 1; + +namespaces_3=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list'); +echo $namespaces_3; +CompareNamespacesLists "${namespaces_3[@]}" "${namespaces_list_expected[@]}" $server_pid; + + +memstats_3=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select name, replication.data_hash, replication.data_count from #memstats order by name'); +CompareMemstats "${memstats_3[@]}" "${memstats_expected[@]}" $server_pid; + +KillAndRemoveServer $server_pid; + +echo "## installing latest release: $LATEST_RELEASE" +yum install -y $LATEST_RELEASE > /dev/null; +reindexer_server -l warning --httplog=none --rpclog=none --db $DB_PATH & +server_pid=$! +sleep 2; + +WaitForDB + +namespaces_4=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list'); +echo $namespaces_4; +CompareNamespacesLists "${namespaces_4[@]}" "${namespaces_3[@]}" $server_pid; + +memstats_4=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select name, replication.data_hash, replication.data_count from #memstats order by name'); +CompareMemstats "${memstats_4[@]}" "${memstats_3[@]}" $server_pid; + +KillAndRemoveServer $server_pid; +rm -rf $DB_PATH; diff --git a/cpp_src/cmd/reindexer_tool/CMakeLists.txt b/cpp_src/cmd/reindexer_tool/CMakeLists.txt index 4b39f2425..f13bcbbc1 100644 --- a/cpp_src/cmd/reindexer_tool/CMakeLists.txt +++ b/cpp_src/cmd/reindexer_tool/CMakeLists.txt @@ -22,7 +22,7 @@ if (NOT MSVC AND NOT WITH_STDLIB_DEBUG) ExternalProject_Add( replxx_lib GIT_REPOSITORY "https://github.com/Restream/replxx" - GIT_TAG "b50b7b7a8c2835b45607cffabc18e4742072e9e6" + GIT_TAG "98aa91965d7495e030f31c6f05969177fe5ab81d" CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${CMAKE_CURRENT_BINARY_DIR} ) include_directories(${CMAKE_CURRENT_BINARY_DIR}/include) diff --git a/cpp_src/core/cbinding/reindexer_c.cc b/cpp_src/core/cbinding/reindexer_c.cc index 0aa64b49b..25f9fa684 100644 --- a/cpp_src/core/cbinding/reindexer_c.cc +++ b/cpp_src/core/cbinding/reindexer_c.cc @@ -1,8 +1,7 @@ #include "reindexer_c.h" -#include #include -#include +#include #include #include "cgocancelcontextpool.h" @@ -16,7 +15,7 @@ using namespace reindexer; const int kQueryResultsPoolSize = 1024; -const int kMaxConcurentQueries = 65534; +const int kMaxConcurrentQueries = 65534; const size_t kCtxArrSize = 1024; const size_t kWarnLargeResultsLimit = 0x40000000; const size_t kMaxPooledResultsCap = 0x10000; @@ -56,7 +55,7 @@ struct TransactionWrapper { }; static std::atomic serializedResultsCount{0}; -static sync_pool res_pool; +static sync_pool res_pool; static CGOCtxPool ctx_pool(kCtxArrSize); struct put_results_to_pool { @@ -94,7 +93,7 @@ static void results2c(std::unique_ptr result, struct reinde out->len = result->ser.Len(); out->data = uintptr_t(result->ser.Buf()); out->results_ptr = uintptr_t(result.release()); - if (const auto count{serializedResultsCount.fetch_add(1, std::memory_order_relaxed)}; count > kMaxConcurentQueries) { + if (const auto count{serializedResultsCount.fetch_add(1, std::memory_order_relaxed)}; count > kMaxConcurrentQueries) { logPrintf(LogWarning, "Too many serialized results: count=%d, alloced=%d", count, res_pool.Alloced()); } } @@ -123,7 +122,7 @@ reindexer_error reindexer_ping(uintptr_t rx) { return error2c(db ? Error(errOK) : err_not_init); } -static void procces_packed_item(Item& item, int mode, int state_token, reindexer_buffer data, const std::vector& precepts, +static void proccess_packed_item(Item& item, int mode, int state_token, reindexer_buffer data, const std::vector& precepts, int format, Error& err) { if (item.Status().ok()) { switch (format) { @@ -151,7 +150,7 @@ static void procces_packed_item(Item& item, int mode, int state_token, reindexer reindexer_error reindexer_modify_item_packed_tx(uintptr_t rx, uintptr_t tr, reindexer_buffer args, reindexer_buffer data) { auto db = reinterpret_cast(rx); - TransactionWrapper* trw = reinterpret_cast(tr); + auto trw = reinterpret_cast(tr); if (!db) { return error2c(err_not_init); } @@ -170,12 +169,12 @@ reindexer_error reindexer_modify_item_packed_tx(uintptr_t rx, uintptr_t tr, rein } Error err = err_not_init; auto item = trw->tr_.NewItem(); - procces_packed_item(item, mode, state_token, data, precepts, format, err); + proccess_packed_item(item, mode, state_token, data, precepts, format, err); if (err.code() == errTagsMissmatch) { item = db->NewItem(trw->tr_.GetName()); err = item.Status(); if (err.ok()) { - procces_packed_item(item, mode, state_token, data, precepts, format, err); + proccess_packed_item(item, mode, state_token, data, precepts, format, err); } } if (err.ok()) { @@ -207,7 +206,7 @@ reindexer_ret reindexer_modify_item_packed(uintptr_t rx, reindexer_buffer args, Item item = rdxKeeper.db().NewItem(ns); - procces_packed_item(item, mode, state_token, data, precepts, format, err); + proccess_packed_item(item, mode, state_token, data, precepts, format, err); const bool needSaveItemValueInQR = !precepts.empty(); query_results_ptr res; @@ -612,7 +611,7 @@ reindexer_ret reindexer_update_query(uintptr_t rx, reindexer_buffer in, reindexe reindexer_error reindexer_delete_query_tx(uintptr_t rx, uintptr_t tr, reindexer_buffer in) { auto db = reinterpret_cast(rx); - TransactionWrapper* trw = reinterpret_cast(tr); + auto trw = reinterpret_cast(tr); if (!db) { return error2c(err_not_init); } @@ -634,7 +633,7 @@ reindexer_error reindexer_delete_query_tx(uintptr_t rx, uintptr_t tr, reindexer_ reindexer_error reindexer_update_query_tx(uintptr_t rx, uintptr_t tr, reindexer_buffer in) { auto db = reinterpret_cast(rx); - TransactionWrapper* trw = reinterpret_cast(tr); + auto trw = reinterpret_cast(tr); if (!db) { return error2c(err_not_init); } @@ -697,7 +696,7 @@ reindexer_ret reindexer_enum_meta(uintptr_t rx, reindexer_string ns, reindexer_c out.len = ser.Len(); out.data = uintptr_t(ser.Buf()); out.results_ptr = uintptr_t(results.release()); - if (const auto count{serializedResultsCount.fetch_add(1, std::memory_order_relaxed)}; count > kMaxConcurentQueries) { + if (const auto count{serializedResultsCount.fetch_add(1, std::memory_order_relaxed)}; count > kMaxConcurrentQueries) { logPrintf(LogWarning, "Too many serialized results: count=%d, alloced=%d", count, res_pool.Alloced()); } } @@ -730,7 +729,7 @@ reindexer_ret reindexer_get_meta(uintptr_t rx, reindexer_string ns, reindexer_st out.len = results->ser.Len(); out.data = uintptr_t(results->ser.Buf()); out.results_ptr = uintptr_t(results.release()); - if (const auto count{serializedResultsCount.fetch_add(1, std::memory_order_relaxed)}; count > kMaxConcurentQueries) { + if (const auto count{serializedResultsCount.fetch_add(1, std::memory_order_relaxed)}; count > kMaxConcurrentQueries) { logPrintf(LogWarning, "Too many serialized results: count=%d, alloced=%d", count, res_pool.Alloced()); } } diff --git a/cpp_src/core/cjson/tagspath.h b/cpp_src/core/cjson/tagspath.h index 116a423bf..ebf9fbd32 100644 --- a/cpp_src/core/cjson/tagspath.h +++ b/cpp_src/core/cjson/tagspath.h @@ -2,11 +2,7 @@ #include #include -#include -#include - -#include "core/keyvalue/key_string.h" -#include "core/keyvalue/variant.h" +#include "estl/h_vector.h" #include "tools/customhash.h" namespace reindexer { @@ -43,16 +39,9 @@ class IndexedPathNode { int NameTag() const noexcept { return nameTag_; } int Index() const noexcept { return index_; } - std::string_view Expression() const noexcept { - if (expression_ && expression_->length() > 0) { - return std::string_view(expression_->c_str(), expression_->length()); - } - return std::string_view(); - } bool IsArrayNode() const noexcept { return (IsForAllItems() || index_ != IndexValueType::NotSet); } bool IsWithIndex() const noexcept { return index_ != ForAllItems && index_ != IndexValueType::NotSet; } - bool IsWithExpression() const noexcept { return expression_ && !expression_->empty(); } bool IsForAllItems() const noexcept { return index_ == ForAllItems; } void MarkAllItems(bool enable) noexcept { @@ -63,14 +52,6 @@ class IndexedPathNode { } } - void SetExpression(std::string_view v) { - if (expression_) { - expression_->assign(v.data(), v.length()); - } else { - expression_ = make_key_string(v.data(), v.length()); - } - } - void SetIndex(int32_t index) noexcept { index_ = index; } void SetNameTag(int16_t nameTag) noexcept { nameTag_ = nameTag; } @@ -78,7 +59,6 @@ class IndexedPathNode { enum : int32_t { ForAllItems = -2 }; int16_t nameTag_ = 0; int32_t index_ = IndexValueType::NotSet; - key_string expression_; }; template diff --git a/cpp_src/core/formatters/key_string_fmt.h b/cpp_src/core/formatters/key_string_fmt.h index 59d0086df..765b78efd 100644 --- a/cpp_src/core/formatters/key_string_fmt.h +++ b/cpp_src/core/formatters/key_string_fmt.h @@ -11,7 +11,7 @@ struct fmt::printf_formatter { } template auto format(const reindexer::key_string& s, ContextT& ctx) const { - return s ? fmt::format_to(ctx.out(), "{}", std::string_view(*s)) : fmt::format_to(ctx.out(), ""); + return s ? fmt::format_to(ctx.out(), "{}", std::string_view(s)) : fmt::format_to(ctx.out(), ""); } }; @@ -19,6 +19,6 @@ template <> struct fmt::formatter : public fmt::formatter { template auto format(const reindexer::key_string& s, ContextT& ctx) const { - return s ? fmt::formatter::format(std::string_view(*s), ctx) : fmt::format_to(ctx.out(), ""); + return s ? fmt::formatter::format(std::string_view(s), ctx) : fmt::format_to(ctx.out(), ""); } }; diff --git a/cpp_src/core/index/indexstore.cc b/cpp_src/core/index/indexstore.cc index 64e8797fa..8bbbe522a 100644 --- a/cpp_src/core/index/indexstore.cc +++ b/cpp_src/core/index/indexstore.cc @@ -29,8 +29,9 @@ void IndexStore::Delete(const Variant& key, IdType /*id*/, StringsHo } assertrx_dbg(keyIt->second > 0); if ((keyIt->second--) == 1) { - const auto strSize = sizeof(*keyIt->first.get()) + keyIt->first->heap_size(); - memStat_.dataSize -= sizeof(unordered_str_map::value_type) + strSize; + const auto strSize = keyIt->first.heap_size(); + const auto staticSizeApproximate = size_t(float(sizeof(unordered_str_map::value_type)) / str_map.max_load_factor()); + memStat_.dataSize -= staticSizeApproximate + strSize; strHolder.Add(std::move(keyIt->first), strSize); str_map.template erase(keyIt); } @@ -70,11 +71,11 @@ Variant IndexStore::Upsert(const Variant& key, IdType id, bool& /*cl keyIt = str_map.find(std::string_view(key)); if (keyIt == str_map.end()) { keyIt = str_map.emplace(static_cast(key), 0).first; - // sizeof(key_string) + heap of string - memStat_.dataSize += sizeof(unordered_str_map::value_type) + sizeof(*keyIt->first.get()) + keyIt->first->heap_size(); + const auto staticSizeApproximate = size_t(float(sizeof(unordered_str_map::value_type)) / str_map.max_load_factor()); + memStat_.dataSize += staticSizeApproximate + keyIt->first.heap_size(); } ++(keyIt->second); - val = (*keyIt->first); + val = keyIt->first; } else { val = std::string_view(key); } diff --git a/cpp_src/core/index/indextext/fieldsgetter.h b/cpp_src/core/index/indextext/fieldsgetter.h index 303392a15..ab26ed7cf 100644 --- a/cpp_src/core/index/indextext/fieldsgetter.h +++ b/cpp_src/core/index/indextext/fieldsgetter.h @@ -1,6 +1,8 @@ #pragma once #include "core/ft/usingcontainer.h" +#include "core/keyvalue/key_string.h" #include "core/payload/fieldsset.h" +#include "core/payload/payloadiface.h" #include "vendor/utf8cpp/utf8/core.h" namespace reindexer { @@ -10,11 +12,11 @@ class FieldsGetter { FieldsGetter(const FieldsSet& fields, const PayloadType& plt, KeyValueType type) : fields_(fields), plt_(plt), type_(type) {} RVector, 8> getDocFields(const key_string& doc, std::vector>&) { - if (!utf8::is_valid(doc->cbegin(), doc->cend())) { + if (!utf8::is_valid(doc.cbegin(), doc.cend())) { throw Error(errParams, "Invalid UTF8 string in FullText index"); } - return {{std::string_view(*doc.get()), 0}}; + return {{std::string_view(doc), 0}}; } VariantArray krefs; diff --git a/cpp_src/core/index/indexunordered.cc b/cpp_src/core/index/indexunordered.cc index 640871f75..96493c972 100644 --- a/cpp_src/core/index/indexunordered.cc +++ b/cpp_src/core/index/indexunordered.cc @@ -134,12 +134,12 @@ size_t heap_size(const key_type& /*kt*/) { template <> size_t heap_size(const key_string& kt) { - return kt->heap_size() + sizeof(*kt.get()); + return kt.heap_size(); } template <> size_t heap_size(const key_string_with_hash& kt) { - return kt->heap_size() + sizeof(*kt.get()); + return kt.heap_size(); } struct DeepClean { diff --git a/cpp_src/core/index/string_map.h b/cpp_src/core/index/string_map.h index 2f7d0a616..5ed267fa8 100644 --- a/cpp_src/core/index/string_map.h +++ b/cpp_src/core/index/string_map.h @@ -14,13 +14,13 @@ struct less_key_string { less_key_string(const CollateOpts& collateOpts = CollateOpts()) : collateOpts_(collateOpts) {} bool operator()(const key_string& lhs, const key_string& rhs) const noexcept { - return collateCompare(*lhs, *rhs, collateOpts_) == ComparationResult::Lt; + return collateCompare(lhs, rhs, collateOpts_) == ComparationResult::Lt; } bool operator()(std::string_view lhs, const key_string& rhs) const noexcept { - return collateCompare(lhs, *rhs, collateOpts_) == ComparationResult::Lt; + return collateCompare(lhs, rhs, collateOpts_) == ComparationResult::Lt; } bool operator()(const key_string& lhs, std::string_view rhs) const noexcept { - return collateCompare(*lhs, rhs, collateOpts_) == ComparationResult::Lt; + return collateCompare(lhs, rhs, collateOpts_) == ComparationResult::Lt; } CollateOpts collateOpts_; }; @@ -29,7 +29,7 @@ class key_string_with_hash : public key_string { public: key_string_with_hash() noexcept : key_string() {} key_string_with_hash(key_string s, CollateMode cm) - : key_string(std::move(s)), hash_(collateHash(**static_cast(this), cm)) {} + : key_string(std::move(s)), hash_(collateHash(*static_cast(this), cm)) {} key_string_with_hash(const key_string_with_hash& o) noexcept : key_string(o), hash_(o.hash_) {} key_string_with_hash(key_string_with_hash&& o) noexcept : key_string(std::move(o)), hash_(o.hash_) {} key_string_with_hash& operator=(key_string_with_hash&& o) noexcept { @@ -47,13 +47,13 @@ struct equal_key_string { equal_key_string(const CollateOpts& collateOpts = CollateOpts()) : collateOpts_(collateOpts) {} bool operator()(const key_string& lhs, const key_string& rhs) const noexcept { - return collateCompare(*lhs, *rhs, collateOpts_) == ComparationResult::Eq; + return collateCompare(lhs, rhs, collateOpts_) == ComparationResult::Eq; } bool operator()(std::string_view lhs, const key_string& rhs) const noexcept { - return collateCompare(lhs, *rhs, collateOpts_) == ComparationResult::Eq; + return collateCompare(lhs, rhs, collateOpts_) == ComparationResult::Eq; } bool operator()(const key_string& lhs, std::string_view rhs) const noexcept { - return collateCompare(*lhs, rhs, collateOpts_) == ComparationResult::Eq; + return collateCompare(lhs, rhs, collateOpts_) == ComparationResult::Eq; } private: @@ -64,7 +64,7 @@ struct hash_key_string { using is_transparent = void; hash_key_string(CollateMode collateMode = CollateNone) noexcept : collateMode_(collateMode) {} - size_t operator()(const key_string& s) const noexcept { return collateHash(*s, collateMode_); } + size_t operator()(const key_string& s) const noexcept { return collateHash(s, collateMode_); } size_t operator()(std::string_view s) const noexcept { return collateHash(s, collateMode_); } size_t operator()(const key_string_with_hash& s) const noexcept { return s.GetHash(); } diff --git a/cpp_src/core/keyvalue/key_string.cc b/cpp_src/core/keyvalue/key_string.cc new file mode 100644 index 000000000..10c6cc4ea --- /dev/null +++ b/cpp_src/core/keyvalue/key_string.cc @@ -0,0 +1,10 @@ +#include "key_string.h" +#include "tools/errors.h" + +namespace reindexer { + +void key_string::throwMaxLenOverflow(size_t len) { + throw Error(errParams, "Key_string length overflow: %d > max key_string length (%d)", len, kMaxLen); +} + +} // namespace reindexer diff --git a/cpp_src/core/keyvalue/key_string.h b/cpp_src/core/keyvalue/key_string.h index 94b678fec..e0205b614 100644 --- a/cpp_src/core/keyvalue/key_string.h +++ b/cpp_src/core/keyvalue/key_string.h @@ -1,114 +1,142 @@ #pragma once +#include #include -#include +#include +#include +#include #include +#include "estl/defines.h" #include "estl/fast_hash_traits.h" -#include "estl/intrusive_ptr.h" namespace reindexer { -typedef const std::string const_string; - -class base_key_string : public std::string { +class key_string_impl { public: - base_key_string(std::string_view str) : std::string(str.data(), str.length()) { - export_hdr_.refcounter.store(0, std::memory_order_release); - bind(); - } - template - base_key_string(Args&&... args) : std::string(std::forward(args)...) { - export_hdr_.refcounter.store(0, std::memory_order_release); - bind(); - } + using size_type = int32_t; - template - void assign(Args&&... args) { - const_string::assign(std::forward(args)...); - bind(); - } - static ptrdiff_t export_hdr_offset() noexcept { - static base_key_string sample; - return ptrdiff_t(reinterpret_cast(&sample.export_hdr_) - reinterpret_cast(&sample)); - } - size_t heap_size() noexcept { - // Check for SSO (small string optimization) - uintptr_t pstart = uintptr_t(this); - uintptr_t pend = pstart + sizeof(std::string); - uintptr_t pdata = uintptr_t(data()); - return (pdata >= pstart && pdata < pend) ? 0 : (capacity() + 1); // +1 for terminating \0 - } + key_string_impl(const key_string_impl&) = delete; + key_string_impl(key_string_impl&&) = delete; + key_string_impl& operator=(const key_string_impl&) = delete; + key_string_impl& operator=(key_string_impl&&) = delete; + + static ptrdiff_t export_hdr_offset() noexcept { return 0; } + const char* data() const noexcept { return data_; } + size_t size() const noexcept { return export_hdr_.len; } + operator std::string_view() const noexcept { return std::string_view(data_, export_hdr_.len); } - // delete all modification methods - to be sure, that base_key_string is mutable, and export will not invalidate after construction - iterator begin() = delete; - iterator end() = delete; - char& operator[](int) = delete; - template - void insert(Args&&... args) = delete; - template - void append(Args&&... args) = delete; - template - void copy(Args&&... args) = delete; - template - void replace(Args&&... args) = delete; - void push_back(char c) = delete; - template - void erase(Args&&... args) = delete; - template - void reserve(Args&&... args) = delete; - template - void resize(Args&&... args) = delete; - void at(int) = delete; - void shrink_to_fit() = delete; - void clear() = delete; - -protected: - friend void intrusive_ptr_add_ref(base_key_string* x) noexcept { + // Unsafe ref counter methods for direct payload access + static void addref_unsafe(const key_string_impl* x) noexcept { if (x) { x->export_hdr_.refcounter.fetch_add(1, std::memory_order_relaxed); } } - friend void intrusive_ptr_release(base_key_string* x) noexcept { - if (x && x->export_hdr_.refcounter.fetch_sub(1, std::memory_order_acq_rel) == 1) { - delete x; // NOLINT(*.NewDelete) False positive + static void release_unsafe(const key_string_impl* x) noexcept { + if ((x && x->export_hdr_.refcounter.fetch_sub(1, std::memory_order_acq_rel) == 1)) { + x->~key_string_impl(); + operator delete(const_cast(x)); } } - friend bool intrusive_ptr_is_unique(base_key_string* x) noexcept { - // std::memory_order_acquire - is essential for COW constructions based on intrusive_ptr - return !x || (x->export_hdr_.refcounter.load(std::memory_order_acquire) == 1); - } - void bind() noexcept { - export_hdr_.cstr = std::string::c_str(); - export_hdr_.len = length(); +private: + friend class key_string; + // Only key_string should be able to construct key_string_impl + explicit key_string_impl(std::string_view str) noexcept { + std::memcpy(data_, str.data(), str.size()); + export_hdr_.cstr = data_; + export_hdr_.len = str.size(); + export_hdr_.refcounter.store(0, std::memory_order_relaxed); } struct export_hdr { const void* cstr; - int32_t len; - std::atomic refcounter; + size_type len; + mutable std::atomic refcounter; } export_hdr_; + char data_[]; }; static_assert(sizeof(std::atomic) == sizeof(int8_t[4]), "refcounter in cbinding (struct reindexer_string) is reserved via int8_t array. Sizes must be same"); -class key_string : public intrusive_ptr { +class key_string { public: - using intrusive_ptr::intrusive_ptr; + using const_iterator = const char*; + using iterator = const_iterator; + + key_string() noexcept : impl_(nullptr) {} + explicit key_string(std::nullptr_t) noexcept : key_string() {} + key_string(const key_string_impl* str) noexcept : impl_(str) { key_string_impl::addref_unsafe(impl_); } + key_string(const key_string_impl* str, bool add_ref) noexcept : impl_(str) { + if (add_ref) { + key_string_impl::addref_unsafe(impl_); + } + } + explicit key_string(std::string_view str) { + if rx_unlikely (str.size() > kMaxLen) { + throwMaxLenOverflow(str.size()); + } + void* impl = operator new(sizeof(key_string_impl) + str.size()); + impl_ = new (impl) key_string_impl(str); + key_string_impl::addref_unsafe(impl_); + } + key_string(const key_string& rhs) noexcept : impl_(rhs.impl_) { key_string_impl::addref_unsafe(impl_); } + key_string(key_string&& rhs) noexcept : impl_(rhs.impl_) { rhs.impl_ = nullptr; } + ~key_string() { key_string_impl::release_unsafe(impl_); } + + key_string& operator=(key_string&& rhs) noexcept { + swap(rhs); + return *this; + } + // NOLINTNEXTLINE(bugprone-unhandled-self-assignment) + key_string& operator=(const key_string& rhs) noexcept { + key_string copy(rhs); + swap(copy); + return *this; + } + + const key_string_impl* get() const noexcept { return impl_; } + size_t size() const noexcept { return impl_ ? impl_->size() : 0; } + const char* data() const& noexcept { return impl_ ? impl_->data() : nullptr; } + const char* data() && = delete; + + explicit operator bool() const noexcept { return impl_; } + operator std::string_view() const noexcept { return impl_ ? std::string_view(*impl_) : std::string_view(); } + void swap(key_string& rhs) noexcept { std::swap(impl_, rhs.impl_); } + size_t heap_size() const noexcept { return impl_ ? (sizeof(key_string_impl) + impl_->size()) : 0; } + + iterator begin() const& noexcept { return impl_ ? impl_->data() : nullptr; } + iterator end() const& noexcept { return impl_ ? (impl_->data() + impl_->size()) : nullptr; } + const_iterator cbegin() const& noexcept { return begin(); } + const_iterator cend() const& noexcept { return end(); } + iterator begin() const&& = delete; + iterator end() && = delete; + const_iterator cbegin() && = delete; + const_iterator cend() && = delete; + +private: + constexpr static size_t kMaxLen = std::numeric_limits::max(); + + [[noreturn]] void throwMaxLenOverflow(size_t len); + + const key_string_impl* impl_; }; template key_string make_key_string(Args&&... args) { - return key_string(new base_key_string(std::forward(args)...)); + return key_string(std::string_view(std::forward(args)...)); } -inline static bool operator==(const key_string& rhs, const key_string& lhs) noexcept { return *rhs == *lhs; } +template +T& operator<<(T& os, const key_string& k) { + return os << std::string_view(k); +} -// Unchecked cast to derived class! -// It assumes, that all strings in payload are intrusive_ptr -inline void key_string_add_ref(std::string* str) noexcept { intrusive_ptr_add_ref(reinterpret_cast(str)); } -inline void key_string_release(std::string* str) noexcept { intrusive_ptr_release(reinterpret_cast(str)); } +inline static bool operator==(const key_string& lhs, const key_string& rhs) noexcept { + return std::string_view(rhs) == std::string_view(lhs); +} +inline static bool operator==(const key_string& lhs, std::string_view rhs) noexcept { return std::string_view(lhs) == rhs; } +inline static bool operator==(std::string_view lhs, const key_string& rhs) noexcept { return std::string_view(rhs) == lhs; } template <> struct is_recommends_sc_hash_map { @@ -116,17 +144,12 @@ struct is_recommends_sc_hash_map { }; } // namespace reindexer -namespace std { -template <> -struct hash { -public: - size_t operator()(const reindexer::base_key_string& obj) const { return hash()(obj); } -}; +namespace std { template <> struct hash { public: - size_t operator()(const reindexer::key_string& obj) const { return hash()(*obj); } + size_t operator()(const reindexer::key_string& obj) const noexcept { return hash()(obj); } }; } // namespace std diff --git a/cpp_src/core/keyvalue/p_string.h b/cpp_src/core/keyvalue/p_string.h index 985a3df9b..1cc608508 100644 --- a/cpp_src/core/keyvalue/p_string.h +++ b/cpp_src/core/keyvalue/p_string.h @@ -39,7 +39,7 @@ struct p_string { constexpr static uint64_t tagVstr = 0x3ULL; // ptr points to slice object constexpr static uint64_t tagSlice = 0x4ULL; - // ptr points to key_string payload atomic_rc_wrapper + // ptr points to key_string payload atomic_rc_wrapper constexpr static uint64_t tagKeyString = 0x5ULL; // ptr points to json_string constexpr static uint64_t tagJsonStr = 0x6ULL; @@ -70,8 +70,9 @@ struct p_string { return std::string_view(str.ptr, str.size); } case tagCxxstr: - case tagKeyString: return std::string_view(*reinterpret_cast(ptr())); + case tagKeyString: + return std::string_view(*reinterpret_cast(ptr())); case tagSlice: return *reinterpret_cast(ptr()); case tagLstr: { @@ -95,8 +96,9 @@ struct p_string { case tagCstr: return reinterpret_cast(ptr()); case tagCxxstr: - case tagKeyString: return (reinterpret_cast(ptr()))->data(); + case tagKeyString: + return (reinterpret_cast(ptr()))->data(); case tagMsgPackStr: return (reinterpret_cast(ptr()))->ptr; case tagSlice: @@ -123,8 +125,9 @@ struct p_string { case tagCstr: return strlen(reinterpret_cast(ptr())); case tagCxxstr: - case tagKeyString: return (reinterpret_cast(ptr()))->length(); + case tagKeyString: + return (reinterpret_cast(ptr()))->size(); case tagSlice: return (reinterpret_cast(ptr()))->size(); case tagLstr: @@ -157,15 +160,19 @@ struct p_string { bool operator>=(p_string other) const noexcept { return compare(other) >= 0; } bool operator<=(p_string other) const noexcept { return compare(other) <= 0; } const std::string* getCxxstr() const noexcept { - assertrx(type() == tagCxxstr || type() == tagKeyString); + assertrx(type() == tagCxxstr); return reinterpret_cast(ptr()); } key_string getKeyString() const noexcept { assertrx(type() == tagKeyString); - auto str = reinterpret_cast(const_cast(ptr())); + auto str = reinterpret_cast(const_cast(ptr())); return key_string(str); } + const key_string_impl* getBaseKeyString() const noexcept { + assertrx(type() == tagKeyString); + return reinterpret_cast(const_cast(ptr())); + } int type() const noexcept { return (v & tagMask) >> tagShift; } std::string toString() const { return std::string(data(), length()); } diff --git a/cpp_src/core/keyvalue/variant.cc b/cpp_src/core/keyvalue/variant.cc index 9e645d481..e22c54306 100644 --- a/cpp_src/core/keyvalue/variant.cc +++ b/cpp_src/core/keyvalue/variant.cc @@ -158,13 +158,7 @@ std::string Variant::As() const { [&](KeyValueType::Bool) { return variant_.value_bool ? "true"s : "false"s; }, [&](KeyValueType::Int64) { return std::to_string(variant_.value_int64); }, [&](KeyValueType::Double) { return double_to_str(variant_.value_double); }, - [&](KeyValueType::String) { - const auto pstr = this->operator p_string(); - if (pstr.type() == p_string::tagCxxstr || pstr.type() == p_string::tagKeyString) { - return *(pstr.getCxxstr()); - } - return pstr.toString(); - }, + [&](KeyValueType::String) { return this->operator p_string().toString(); }, [&](KeyValueType::Null) { return "null"s; }, [this](OneOf) -> std::string { throw Error(errParams, "Can't convert '%s'-value to string", variant_.type.Name()); @@ -992,15 +986,16 @@ void Variant::convertToComposite(const PayloadType& payloadType, const FieldsSet } // Alloc usual payloadvalue + extra memory for hold string - auto& pv = *new (cast()) PayloadValue(payloadType.TotalSize() + val->size()); + auto strSz = val.size(); + auto& pv = *new (cast()) PayloadValue(payloadType.TotalSize() + strSz); variant_.hold = 1; variant_.type = KeyValueType::Composite{}; // Copy serializer buffer with strings to extra payloadvalue memory char* data = reinterpret_cast(pv.Ptr() + payloadType.TotalSize()); - memcpy(data, val->data(), val->size()); + memcpy(data, val.data(), strSz); - Serializer ser(std::string_view(data, val->size())); + Serializer ser(std::string_view(data, strSz)); size_t count = ser.GetVarUint(); if (count != fields.size()) { @@ -1026,7 +1021,7 @@ VariantArray Variant::getCompositeValues() const { assertrx(variant_.type.Is()); VariantArray res; - Serializer ser(**cast()); + Serializer ser(*cast()); size_t count = ser.GetVarUint(); res.reserve(count); while (count--) { @@ -1056,7 +1051,7 @@ Variant::operator p_string() const noexcept { Variant::operator std::string_view() const noexcept { assertrx(!isUuid()); assertKeyType(variant_.type); - return (variant_.hold == 1) ? std::string_view(**cast()) : *cast(); + return (variant_.hold == 1) ? std::string_view(*cast()) : *cast(); } Variant::operator const PayloadValue&() const noexcept { assertrx(!isUuid()); diff --git a/cpp_src/core/keyvalue/variant.h b/cpp_src/core/keyvalue/variant.h index 9c147f2bb..34fc0954f 100644 --- a/cpp_src/core/keyvalue/variant.h +++ b/cpp_src/core/keyvalue/variant.h @@ -40,9 +40,6 @@ class Variant { Variant(p_string v, hold_t); explicit Variant(p_string v) noexcept : Variant(v, no_hold_t{}) {} explicit Variant(const std::string& v) : variant_{0, 1, KeyValueType::String{}} { new (cast()) key_string(make_key_string(v)); } - explicit Variant(std::string&& v) : variant_{0, 1, KeyValueType::String{}} { - new (cast()) key_string(make_key_string(std::move(v))); - } explicit Variant(std::string_view v) : variant_{0, 1, KeyValueType::String{}} { new (cast()) key_string(make_key_string(v)); } explicit Variant(const key_string& v) noexcept : variant_{0, 1, KeyValueType::String{}} { new (cast()) key_string(v); } explicit Variant(key_string&& v) noexcept : variant_{0, 1, KeyValueType::String{}} { new (cast()) key_string(std::move(v)); } diff --git a/cpp_src/core/namespace/stringsholder.h b/cpp_src/core/namespace/stringsholder.h index a0fa9cd68..511cfa9ef 100644 --- a/cpp_src/core/namespace/stringsholder.h +++ b/cpp_src/core/namespace/stringsholder.h @@ -3,6 +3,7 @@ #include #include #include "core/keyvalue/key_string.h" +#include "estl/intrusive_ptr.h" namespace reindexer { @@ -14,22 +15,22 @@ class StringsHolder : private std::vector { public: ~StringsHolder(); void Add(key_string&& str, size_t strSize) { - memStat_ += sizeof(Base::value_type) + strSize; Base::emplace_back(std::move(str)); + memStat_ += sizeof(Base::value_type) + strSize; } void Add(key_string&& str) { - memStat_ += sizeof(Base::value_type) + sizeof(*str.get()) + str->heap_size(); - Base::emplace_back(std::move(str)); + auto& s = Base::emplace_back(std::move(str)); + memStat_ += sizeof(Base::value_type) + s.heap_size(); } void Add(const key_string& str) { - memStat_ += sizeof(Base::value_type) + sizeof(*str.get()) + str->heap_size(); - Base::push_back(str); + auto& s = Base::emplace_back(str); + memStat_ += sizeof(Base::value_type) + s.heap_size(); } void Add(std::unique_ptr&&); template void emplace_back(T&&... v) { - Base::emplace_back(std::forward(v)...); - memStat_ += sizeof(Base::value_type) + sizeof(*back().get()) + back()->heap_size(); + auto& str = Base::emplace_back(std::forward(v)...); + memStat_ += sizeof(key_string) + str.heap_size(); } void Clear() noexcept; size_t MemStat() const noexcept { return memStat_; } diff --git a/cpp_src/core/nsselecter/comparator/comparator_indexed.h b/cpp_src/core/nsselecter/comparator/comparator_indexed.h index 355f82864..60b24d49c 100644 --- a/cpp_src/core/nsselecter/comparator/comparator_indexed.h +++ b/cpp_src/core/nsselecter/comparator/comparator_indexed.h @@ -29,7 +29,7 @@ template struct ValuesHolder { struct Type { Type() noexcept = default; - Type(key_string v) noexcept : value_{std::move(v)}, valueView_{value_ ? std::string_view{*value_} : std::string_view{}} {} + Type(key_string v) noexcept : value_{std::move(v)}, valueView_{value_} {} Type(const Type& other) noexcept : Type{other.value_} {} Type(Type&& other) noexcept : Type{std::move(other.value_)} { other.valueView_ = {}; } Type& operator=(const Type& other) noexcept { diff --git a/cpp_src/core/nsselecter/comparator/equalposition_comparator_impl.h b/cpp_src/core/nsselecter/comparator/equalposition_comparator_impl.h index eb31bebbc..3f0c24605 100644 --- a/cpp_src/core/nsselecter/comparator/equalposition_comparator_impl.h +++ b/cpp_src/core/nsselecter/comparator/equalposition_comparator_impl.h @@ -217,7 +217,7 @@ class EqualPositionComparatorTypeImpl { return collateCompare(std::string_view(lhs), rhs, collate_) == ComparationResult::Gt; case CondRange: return (collateCompare(std::string_view(lhs), rhs, collate_) & ComparationResult::Ge) && - (collateCompare(std::string_view(lhs), std::string_view(*values_[1]), collate_) & ComparationResult::Le); + (collateCompare(std::string_view(lhs), std::string_view(values_[1]), collate_) & ComparationResult::Le); case CondSet: return valuesS_.find(std::string_view(lhs)) != valuesS_.end(); case CondAllSet: { @@ -263,7 +263,7 @@ class EqualPositionComparatorTypeImpl { } else { values_.emplace_back(value); if (values_.size() == 1) { - cachedValueSV_ = std::string_view(*values_[0]); + cachedValueSV_ = std::string_view(values_[0]); } } } diff --git a/cpp_src/core/payload/fieldsset.h b/cpp_src/core/payload/fieldsset.h index f289448c9..1a37cedd2 100644 --- a/cpp_src/core/payload/fieldsset.h +++ b/cpp_src/core/payload/fieldsset.h @@ -6,6 +6,8 @@ #include "core/cjson/tagspath.h" #include "core/type_consts.h" #include "estl/h_vector.h" +#include "estl/overloaded.h" +#include "tools/assertrx.h" namespace reindexer { diff --git a/cpp_src/core/payload/payloadiface.cc b/cpp_src/core/payload/payloadiface.cc index 207a4d07d..886092877 100644 --- a/cpp_src/core/payload/payloadiface.cc +++ b/cpp_src/core/payload/payloadiface.cc @@ -526,16 +526,19 @@ template void PayloadIface::AddRefStrings(int field) noexcept { auto& f = t_.Field(field); assertrx(f.Type().template Is()); + auto vptr = v_->Ptr(); // direct payloadvalue manipulation for speed optimize if (!f.IsArray()) { - auto str = *reinterpret_cast((v_->Ptr() + f.Offset())); - key_string_add_ref(const_cast(str.getCxxstr())); + auto str = *reinterpret_cast((vptr + f.Offset())); + key_string_impl::addref_unsafe(str.getBaseKeyString()); } else { - auto arr = reinterpret_cast(v_->Ptr() + f.Offset()); - for (int i = 0; i < arr->len; i++) { - auto str = *reinterpret_cast(v_->Ptr() + arr->offset + i * t_.Field(field).ElemSizeof()); - key_string_add_ref(const_cast(str.getCxxstr())); + const auto elemSize = f.ElemSizeof(); + auto arr = reinterpret_cast(vptr + f.Offset()); + const auto arrOffset = arr->offset; + for (int i = 0, arrLen = arr->len; i < arrLen; ++i) { + auto str = reinterpret_cast(vptr + arrOffset + i * elemSize); + key_string_impl::addref_unsafe(str->getBaseKeyString()); } } } @@ -551,16 +554,19 @@ template void PayloadIface::ReleaseStrings(int field) noexcept { auto& f = t_.Field(field); assertrx(f.Type().template Is()); + auto vptr = v_->Ptr(); // direct payloadvalue manipulation for speed optimize if (!f.IsArray()) { - auto str = *reinterpret_cast((v_->Ptr() + f.Offset())); - key_string_release(const_cast(str.getCxxstr())); + auto str = reinterpret_cast((vptr + f.Offset())); + key_string_impl::release_unsafe(str->getBaseKeyString()); } else { - auto arr = reinterpret_cast(v_->Ptr() + f.Offset()); - for (int i = 0; i < arr->len; i++) { - auto str = *reinterpret_cast(v_->Ptr() + arr->offset + i * t_.Field(field).ElemSizeof()); - key_string_release(const_cast(str.getCxxstr())); + const auto elemSize = f.ElemSizeof(); + auto arr = reinterpret_cast(vptr + f.Offset()); + const auto arrOffset = arr->offset; + for (int i = 0, arrLen = arr->len; i < arrLen; ++i) { + auto str = reinterpret_cast(vptr + arrOffset + i * elemSize); + key_string_impl::release_unsafe(str->getBaseKeyString()); } } } @@ -574,12 +580,12 @@ void PayloadIface::copyOrMoveStrings(int field, StrHolder& dest, bool copy) { // direct payloadvalue manipulation for speed optimize if (!f.IsArray()) { auto str = *reinterpret_cast((v_->Ptr() + f.Offset())); - dest.emplace_back(reinterpret_cast(const_cast(str.getCxxstr())), copy); + dest.emplace_back(str.getBaseKeyString(), copy); } else { auto arr = reinterpret_cast(v_->Ptr() + f.Offset()); for (int i = 0; i < arr->len; i++) { auto str = *reinterpret_cast(v_->Ptr() + arr->offset + i * t_.Field(field).ElemSizeof()); - dest.emplace_back(reinterpret_cast(const_cast(str.getCxxstr())), copy); + dest.emplace_back(str.getBaseKeyString(), copy); } } } @@ -609,14 +615,14 @@ void PayloadIface::MoveStrings(int field, StringsHolder& dest) { template void PayloadIface::CopyStrings(std::vector& dest) { - for (auto field : t_.StrFields()) { + for (int field : t_.StrFields()) { copyOrMoveStrings(field, dest, true); } } template void PayloadIface::ReleaseStrings() noexcept { - for (auto field : t_.StrFields()) { + for (int field : t_.StrFields()) { ReleaseStrings(field); } } diff --git a/cpp_src/core/payload/payloadtype.cc b/cpp_src/core/payload/payloadtype.cc index 2cc553053..c0ffb0c2d 100644 --- a/cpp_src/core/payload/payloadtype.cc +++ b/cpp_src/core/payload/payloadtype.cc @@ -151,7 +151,7 @@ int PayloadTypeImpl::FieldByJsonPath(std::string_view jsonPath) const noexcept { } void PayloadTypeImpl::serialize(WrSerializer& ser) const { - ser.PutVarUint(base_key_string::export_hdr_offset()); + ser.PutVarUint(key_string_impl::export_hdr_offset()); ser.PutVarUint(NumFields()); for (int i = 0; i < NumFields(); i++) { ser.PutKeyValueType(Field(i).Type()); diff --git a/cpp_src/core/selectfunc/functions/debugrank.cc b/cpp_src/core/selectfunc/functions/debugrank.cc index cee1d5b74..68ad444d7 100644 --- a/cpp_src/core/selectfunc/functions/debugrank.cc +++ b/cpp_src/core/selectfunc/functions/debugrank.cc @@ -40,7 +40,7 @@ bool DebugRank::Process(ItemRef& res, PayloadType& plType, const SelectFuncStruc throw Error(errLogic, "Unable to apply debug_rank function to the non-string field '%s'", func.field); } - const std::string* data = p_string(kr[0]).getCxxstr(); + const std::string_view data = std::string_view(p_string(kr[0])); const auto pva = dataFtCtx.area[it->second].GetAreas(func.fieldNo); if (!pva || pva->Empty()) { @@ -51,7 +51,7 @@ bool DebugRank::Process(ItemRef& res, PayloadType& plType, const SelectFuncStruc std::string resultString; auto splitterTask = ftctx->GetData()->splitter->CreateTask(); - splitterTask->SetText(*data); + splitterTask->SetText(data); static const std::string_view startString = ""; static const std::string_view endString = ""; @@ -63,7 +63,7 @@ bool DebugRank::Process(ItemRef& res, PayloadType& plType, const SelectFuncStruc bool next = false; int endStringCount = 0; std::pair pos = splitterTask->Convert(areaVector[id].start, areaVector[id].end); - resultString += std::string_view(data->c_str() + beforeStr, pos.first - beforeStr); + resultString += std::string_view(data.data() + beforeStr, pos.first - beforeStr); do { next = false; switch (areaVector[id].phraseMode) { @@ -86,13 +86,13 @@ bool DebugRank::Process(ItemRef& res, PayloadType& plType, const SelectFuncStruc next = true; } } while (next); - resultString += std::string_view(data->c_str() + pos.first, pos.second - pos.first); + resultString += std::string_view(data.data() + pos.first, pos.second - pos.first); beforeStr = pos.second; for (int i = 0; i < endStringCount; i++) { resultString += endString; } } - resultString += std::string_view(data->c_str() + beforeStr, data->size() - beforeStr); + resultString += std::string_view(data.data() + beforeStr, data.size() - beforeStr); stringsHolder.emplace_back(make_key_string(std::move(resultString))); res.Value().Clone(); @@ -101,4 +101,4 @@ bool DebugRank::Process(ItemRef& res, PayloadType& plType, const SelectFuncStruc return true; } -} // namespace reindexer \ No newline at end of file +} // namespace reindexer diff --git a/cpp_src/core/selectfunc/functions/highlight.cc b/cpp_src/core/selectfunc/functions/highlight.cc index 08921d3b3..bbc4ac4f6 100644 --- a/cpp_src/core/selectfunc/functions/highlight.cc +++ b/cpp_src/core/selectfunc/functions/highlight.cc @@ -38,7 +38,7 @@ bool Highlight::Process(ItemRef& res, PayloadType& pl_type, const SelectFuncStru throw Error(errLogic, "Unable to apply highlight function to the non-string field '%s'", func.field); } - const std::string* data = p_string(kr[0]).getCxxstr(); + const std::string_view data = std::string_view(p_string(kr[0])); auto pva = dataFtCtx.area[it->second].GetAreas(func.fieldNo); if (!pva || pva->Empty()) { @@ -47,11 +47,11 @@ bool Highlight::Process(ItemRef& res, PayloadType& pl_type, const SelectFuncStru auto& va = *pva; std::string result_string; - result_string.reserve(data->size() + va.Size() * (func.funcArgs[0].size() + func.funcArgs[1].size())); - result_string = *data; + result_string.reserve(data.size() + va.Size() * (func.funcArgs[0].size() + func.funcArgs[1].size())); + result_string.append(data); auto splitterTask = ftctx->GetData()->splitter->CreateTask(); - splitterTask->SetText(*data); + splitterTask->SetText(data); int offset = 0; for (auto area : va.GetData()) { diff --git a/cpp_src/core/selectfunc/functions/snippet.cc b/cpp_src/core/selectfunc/functions/snippet.cc index e92856dc6..a5361a57c 100644 --- a/cpp_src/core/selectfunc/functions/snippet.cc +++ b/cpp_src/core/selectfunc/functions/snippet.cc @@ -100,7 +100,7 @@ void Snippet::init(const SelectFuncStruct& func) { isInit_ = true; } -void Snippet::addSnippet(std::string& resultString, const std::string& data, const Area& snippetAreaPrev, +void Snippet::addSnippet(std::string& resultString, std::string_view data, const Area& snippetAreaPrev, const Area& snippetAreaPrevChar) const { resultString.append(preDelim_); @@ -192,7 +192,7 @@ A Snippet::RecalcZoneHelper::RecalcZoneToOffset(const Area& area) { return outAreas; } -void Snippet::buildResult(RecalcZoneHelper& recalcZoneHelper, const AreasInField& pva, const std::string& data, +void Snippet::buildResult(RecalcZoneHelper& recalcZoneHelper, const AreasInField& pva, std::string_view data, std::string& resultString) { // resultString =preDelim_+with_area_str+data_str_before+marker_before+zone_str+marker_after+data_strAfter+postDelim_ Area snippetAreaPrev; @@ -235,7 +235,7 @@ void Snippet::buildResult(RecalcZoneHelper& recalcZoneHelper, const AreasInField resultString.append(postDelim_); } -void Snippet::buildResultWithPrefix(RecalcZoneHelper& recalcZoneHelper, const AreasInField& pva, const std::string& data, +void Snippet::buildResultWithPrefix(RecalcZoneHelper& recalcZoneHelper, const AreasInField& pva, std::string_view data, std::string& resultString) { // resultString =preDelim_+with_area_str+data_str_before+marker_before+zone_str+marker_after+data_strAfter+postDelim_ Area snippetAreaPrev; @@ -296,7 +296,7 @@ bool Snippet::Process(ItemRef& res, PayloadType& plType, const SelectFuncStruct& throw Error(errLogic, "Unable to apply snippet function to the non-string field '%s'", func.field); } - const std::string* data = p_string(kr[0]).getCxxstr(); + const std::string_view data = std::string_view(p_string(kr[0])); auto pva = dataFtCtx.area[it->second].GetAreas(func.fieldNo); if (!pva || pva->Empty()) { @@ -304,14 +304,14 @@ bool Snippet::Process(ItemRef& res, PayloadType& plType, const SelectFuncStruct& } std::string resultString; - resultString.reserve(data->size()); + resultString.reserve(data.size()); - RecalcZoneHelper recalcZoneHelper(*data, ftctx->GetData()->splitter, after_, before_, leftBound_, rightBound_); + RecalcZoneHelper recalcZoneHelper(data, ftctx->GetData()->splitter, after_, before_, leftBound_, rightBound_); if (needAreaStr_) { - buildResultWithPrefix(recalcZoneHelper, *pva, *data, resultString); + buildResultWithPrefix(recalcZoneHelper, *pva, data, resultString); } else { - buildResult(recalcZoneHelper, *pva, *data, resultString); + buildResult(recalcZoneHelper, *pva, data, resultString); } stringsHolder.emplace_back(make_key_string(std::move(resultString))); diff --git a/cpp_src/core/selectfunc/functions/snippet.h b/cpp_src/core/selectfunc/functions/snippet.h index 58a93152a..448bb1d02 100644 --- a/cpp_src/core/selectfunc/functions/snippet.h +++ b/cpp_src/core/selectfunc/functions/snippet.h @@ -14,7 +14,7 @@ class Snippet { private: void init(const SelectFuncStruct& func); - void addSnippet(std::string& resultString, const std::string& data, const Area& snippetAreaPrev, const Area& snippetAreaPrevChar) const; + void addSnippet(std::string& resultString, std::string_view data, const Area& snippetAreaPrev, const Area& snippetAreaPrevChar) const; class RecalcZoneHelper { public: @@ -41,9 +41,8 @@ class Snippet { std::string_view leftBound_, rightBound_; }; - void buildResult(RecalcZoneHelper& recalcZoneHelper, const AreasInField& pva, const std::string& data, std::string& resultString); - - void buildResultWithPrefix(RecalcZoneHelper& recalcZoneHelper, const AreasInField& pva, const std::string& data, + void buildResult(RecalcZoneHelper& recalcZoneHelper, const AreasInField& pva, std::string_view data, std::string& resultString); + void buildResultWithPrefix(RecalcZoneHelper& recalcZoneHelper, const AreasInField& pva, std::string_view data, std::string& resultString); bool isInit_ = false; diff --git a/cpp_src/estl/intrusive_ptr.h b/cpp_src/estl/intrusive_ptr.h index 4c618512b..33ec974c8 100644 --- a/cpp_src/estl/intrusive_ptr.h +++ b/cpp_src/estl/intrusive_ptr.h @@ -1,7 +1,7 @@ #pragma once -#include #include +#include #include "tools/assertrx.h" namespace reindexer { @@ -16,86 +16,57 @@ class intrusive_ptr { constexpr intrusive_ptr() noexcept = default; constexpr intrusive_ptr(std::nullptr_t) noexcept {} - - intrusive_ptr(T* p, bool add_ref = true) noexcept : px(p) { - if (px != 0 && add_ref) { + intrusive_ptr(T* p) noexcept : px(p) { intrusive_ptr_add_ref(px); } + intrusive_ptr(T* p, bool add_ref) noexcept : px(p) { + if (add_ref) { intrusive_ptr_add_ref(px); } } - template intrusive_ptr(const intrusive_ptr& rhs) noexcept : px(rhs.get()) { - if (px != 0) { - intrusive_ptr_add_ref(px); - } - } - - intrusive_ptr(const intrusive_ptr& rhs) noexcept : px(rhs.px) { - if (px != 0) { - intrusive_ptr_add_ref(px); - } - } - - ~intrusive_ptr() { - if (px != 0) { - intrusive_ptr_release(px); - } + intrusive_ptr_add_ref(px); } + intrusive_ptr(const intrusive_ptr& rhs) noexcept : px(rhs.px) { intrusive_ptr_add_ref(px); } + intrusive_ptr(intrusive_ptr&& rhs) noexcept : px(rhs.px) { rhs.px = 0; } + ~intrusive_ptr() { intrusive_ptr_release(px); } template intrusive_ptr& operator=(const intrusive_ptr& rhs) noexcept { this_type(rhs).swap(*this); return *this; } - - intrusive_ptr(intrusive_ptr&& rhs) noexcept : px(rhs.px) { rhs.px = 0; } - - intrusive_ptr& operator=(intrusive_ptr&& rhs) noexcept { - this_type(static_cast(rhs)).swap(*this); - return *this; - } // NOLINTNEXTLINE(bugprone-unhandled-self-assignment) intrusive_ptr& operator=(const intrusive_ptr& rhs) noexcept { this_type(rhs).swap(*this); return *this; } - + intrusive_ptr& operator=(intrusive_ptr&& rhs) noexcept { + this_type(std::move(rhs)).swap(*this); + return *this; + } intrusive_ptr& operator=(T* rhs) noexcept { this_type(rhs).swap(*this); return *this; } void reset() noexcept { this_type().swap(*this); } - void reset(T* rhs) noexcept { this_type(rhs).swap(*this); } - bool unique() const noexcept { - if (px == 0) { - return true; - } - return intrusive_ptr_is_unique(px); - } + bool unique() const noexcept { return intrusive_ptr_is_unique(px); } T* get() const noexcept { return px; } - T& operator*() const noexcept { - assertrx(px != 0); + assertrx_dbg(px != 0); return *px; } - T* operator->() const noexcept { - assertrx(px != 0); + assertrx_dbg(px != 0); return px; } - typedef T* this_type::*unspecified_bool_type; + typedef T* this_type::* unspecified_bool_type; operator unspecified_bool_type() const noexcept { return px == 0 ? 0 : &this_type::px; } - - void swap(intrusive_ptr& rhs) noexcept { - T* tmp = px; - px = rhs.px; - rhs.px = tmp; - } + void swap(intrusive_ptr& rhs) noexcept { std::swap(px, rhs.px); } private: T* px{nullptr}; @@ -276,7 +247,7 @@ class intrusive_rc_base { friend void intrusive_ptr_add_ref(intrusive_rc_base* x) noexcept; friend void intrusive_ptr_release(intrusive_rc_base* x) noexcept; - friend bool intrusive_ptr_is_unique(intrusive_rc_base* x) noexcept; + friend bool intrusive_ptr_is_unique(const intrusive_rc_base* x) noexcept; }; inline void intrusive_ptr_add_ref(intrusive_rc_base* x) noexcept { @@ -291,7 +262,7 @@ inline void intrusive_ptr_release(intrusive_rc_base* x) noexcept { } } -inline bool intrusive_ptr_is_unique(intrusive_rc_base* x) noexcept { return !x || (x->refcount == 1); } +inline bool intrusive_ptr_is_unique(const intrusive_rc_base* x) noexcept { return !x || (x->refcount == 1); } template intrusive_ptr make_intrusive(Args&&... args) { diff --git a/cpp_src/gtests/tests/fixtures/ft_api.cc b/cpp_src/gtests/tests/fixtures/ft_api.cc index 85efb7161..9228a0495 100644 --- a/cpp_src/gtests/tests/fixtures/ft_api.cc +++ b/cpp_src/gtests/tests/fixtures/ft_api.cc @@ -173,10 +173,10 @@ reindexer::Error FTApi::Delete(int id) { return this->rt.reindexer->Delete("nm1", item); } -reindexer::QueryResults FTApi::SimpleCompositeSelect(std::string word) { +reindexer::QueryResults FTApi::SimpleCompositeSelect(std::string_view word) { auto qr{reindexer::Query("nm1").Where("ft3", CondEq, word)}; reindexer::QueryResults res; - auto mqr{reindexer::Query("nm2").Where("ft3", CondEq, std::move(word))}; + auto mqr{reindexer::Query("nm2").Where("ft3", CondEq, word)}; mqr.AddFunction("ft1 = snippet(,\"\",3,2,,d)"); qr.Merge(std::move(mqr)); @@ -187,11 +187,11 @@ reindexer::QueryResults FTApi::SimpleCompositeSelect(std::string word) { return res; } -reindexer::QueryResults FTApi::CompositeSelectField(const std::string& field, std::string word) { - word = '@' + field + ' ' + word; - auto qr{reindexer::Query("nm1").Where("ft3", CondEq, word)}; +reindexer::QueryResults FTApi::CompositeSelectField(const std::string& field, std::string_view word) { + const auto query = fmt::format("@{} {}", field, word); + auto qr{reindexer::Query("nm1").Where("ft3", CondEq, query)}; reindexer::QueryResults res; - auto mqr{reindexer::Query("nm2").Where("ft3", CondEq, std::move(word))}; + auto mqr{reindexer::Query("nm2").Where("ft3", CondEq, query)}; mqr.AddFunction(field + " = snippet(,\"\",3,2,,d)"); qr.Merge(std::move(mqr)); @@ -202,8 +202,8 @@ reindexer::QueryResults FTApi::CompositeSelectField(const std::string& field, st return res; } -reindexer::QueryResults FTApi::StressSelect(std::string word) { - const auto qr{reindexer::Query("nm1").Where("ft3", CondEq, std::move(word))}; +reindexer::QueryResults FTApi::StressSelect(std::string_view word) { + const auto qr{reindexer::Query("nm1").Where("ft3", CondEq, word)}; reindexer::QueryResults res; auto err = rt.reindexer->Select(qr, res); EXPECT_TRUE(err.ok()) << err.what(); diff --git a/cpp_src/gtests/tests/fixtures/ft_api.h b/cpp_src/gtests/tests/fixtures/ft_api.h index b9c4655f3..f91d9a577 100644 --- a/cpp_src/gtests/tests/fixtures/ft_api.h +++ b/cpp_src/gtests/tests/fixtures/ft_api.h @@ -35,9 +35,9 @@ class FTApi : public ::testing::TestWithParam CreateAllPermutatedQueries(const std::string& queryStart, std::vector words, const std::string& queryEnd, const std::string& sep = " "); void CheckAllPermutations(const std::string& queryStart, const std::vector& words, const std::string& queryEnd, diff --git a/cpp_src/gtests/tests/fixtures/queries_verifier.h b/cpp_src/gtests/tests/fixtures/queries_verifier.h index 945ac0384..2e1af35d5 100644 --- a/cpp_src/gtests/tests/fixtures/queries_verifier.h +++ b/cpp_src/gtests/tests/fixtures/queries_verifier.h @@ -2,7 +2,8 @@ #include -#if defined(__GNUC__) && (__GNUC__ == 12) && defined(REINDEX_WITH_ASAN) +#if defined(__GNUC__) && ((__GNUC__ == 12) || (__GNUC__ == 13)) && defined(REINDEX_WITH_ASAN) +// regex header is broken in GCC 12.0-13.3 with ASAN #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wmaybe-uninitialized" #include diff --git a/cpp_src/gtests/tests/unit/composite_indexes_test.cc b/cpp_src/gtests/tests/unit/composite_indexes_test.cc index 28ee5f794..5348932e3 100644 --- a/cpp_src/gtests/tests/unit/composite_indexes_test.cc +++ b/cpp_src/gtests/tests/unit/composite_indexes_test.cc @@ -109,8 +109,8 @@ TEST_F(CompositeIndexesApi, CompositeIndexesDropTest) { TEST_F(CompositeIndexesApi, CompositeIndexesSelectTest) { int priceValue = 77777, pagesValue = 88888; - const char* titleValue = "test book1 title"; - const char* nameValue = "test book1 name"; + constexpr std::string_view titleValue = "test book1 title"; + constexpr std::string_view nameValue = "test book1 name"; addCompositeIndex({kFieldNameBookid, kFieldNameBookid2}, CompositeIndexHash, IndexOpts().PK()); fillNamespace(0, 100); @@ -118,7 +118,7 @@ TEST_F(CompositeIndexesApi, CompositeIndexesSelectTest) { std::string compositeIndexName(getCompositeIndexName({kFieldNamePrice, kFieldNamePages})); addCompositeIndex({kFieldNamePrice, kFieldNamePages}, CompositeIndexHash, IndexOpts()); - addOneRow(300, 3000, titleValue, pagesValue, priceValue, nameValue); + addOneRow(300, 3000, std::string(titleValue), pagesValue, priceValue, std::string(nameValue)); fillNamespace(101, 200); auto qr = execAndCompareQuery( @@ -134,8 +134,8 @@ TEST_F(CompositeIndexesApi, CompositeIndexesSelectTest) { Item titleNameRow = qr.begin().GetItem(false); Variant selectedTitle = titleNameRow[kFieldNameTitle]; Variant selectedName = titleNameRow[kFieldNameName]; - EXPECT_EQ(static_cast(selectedTitle)->compare(std::string(titleValue)), 0); - EXPECT_EQ(static_cast(selectedName)->compare(std::string(nameValue)), 0); + EXPECT_EQ(static_cast(selectedTitle), titleValue); + EXPECT_EQ(static_cast(selectedName), nameValue); execAndCompareQuery(Query(default_namespace).WhereComposite(compositeIndexName, CondLt, {{Variant(priceValue), Variant(pagesValue)}})); execAndCompareQuery(Query(default_namespace).WhereComposite(compositeIndexName, CondLe, {{Variant(priceValue), Variant(pagesValue)}})); @@ -153,7 +153,7 @@ TEST_F(CompositeIndexesApi, CompositeIndexesSelectTest) { for (int i = 0; i < 10; ++i) { intKeys.emplace_back(VariantArray{Variant(i), Variant(i * 5)}); } - execAndCompareQuery(Query(default_namespace).WhereComposite(compositeIndexName.c_str(), CondSet, intKeys)); + execAndCompareQuery(Query(default_namespace).WhereComposite(compositeIndexName, CondSet, intKeys)); dropIndex(compositeIndexName); fillNamespace(401, 500); @@ -163,18 +163,10 @@ TEST_F(CompositeIndexesApi, CompositeIndexesSelectTest) { fillNamespace(701, 900); - execAndCompareQuery( - Query(default_namespace) - .WhereComposite(compositeIndexName2.c_str(), CondEq, {{Variant(std::string(titleValue)), Variant(std::string(nameValue))}})); - execAndCompareQuery( - Query(default_namespace) - .WhereComposite(compositeIndexName2.c_str(), CondGe, {{Variant(std::string(titleValue)), Variant(std::string(nameValue))}})); - execAndCompareQuery( - Query(default_namespace) - .WhereComposite(compositeIndexName2.c_str(), CondLt, {{Variant(std::string(titleValue)), Variant(std::string(nameValue))}})); - execAndCompareQuery( - Query(default_namespace) - .WhereComposite(compositeIndexName2.c_str(), CondLe, {{Variant(std::string(titleValue)), Variant(std::string(nameValue))}})); + execAndCompareQuery(Query(default_namespace).WhereComposite(compositeIndexName2, CondEq, {{Variant(titleValue), Variant(nameValue)}})); + execAndCompareQuery(Query(default_namespace).WhereComposite(compositeIndexName2, CondGe, {{Variant(titleValue), Variant(nameValue)}})); + execAndCompareQuery(Query(default_namespace).WhereComposite(compositeIndexName2, CondLt, {{Variant(titleValue), Variant(nameValue)}})); + execAndCompareQuery(Query(default_namespace).WhereComposite(compositeIndexName2, CondLe, {{Variant(titleValue), Variant(nameValue)}})); fillNamespace(1201, 2000); @@ -182,11 +174,10 @@ TEST_F(CompositeIndexesApi, CompositeIndexesSelectTest) { for (size_t i = 0; i < 1010; ++i) { stringKeys.emplace_back(VariantArray{Variant(RandString()), Variant(RandString())}); } - execAndCompareQuery(Query(default_namespace).WhereComposite(compositeIndexName2.c_str(), CondSet, stringKeys)); - execAndCompareQuery( - Query(default_namespace) - .Where(kFieldNameName, CondEq, nameValue) - .WhereComposite(compositeIndexName2.c_str(), CondEq, {{Variant(std::string(titleValue)), Variant(std::string(nameValue))}})); + execAndCompareQuery(Query(default_namespace).WhereComposite(compositeIndexName2, CondSet, stringKeys)); + execAndCompareQuery(Query(default_namespace) + .Where(kFieldNameName, CondEq, nameValue) + .WhereComposite(compositeIndexName2, CondEq, {{Variant(titleValue), Variant(nameValue)}})); dropIndex(compositeIndexName2); fillNamespace(201, 300); @@ -361,4 +352,4 @@ TEST_F(CompositeIndexesApi, FastUpdateIndex) { ASSERT_THAT(err.what(), testing::MatchesRegex(fmt::format("({}|{})", err1Text, err2Text))); } } -} \ No newline at end of file +} diff --git a/cpp_src/gtests/tests/unit/ft/ft_generic.cc b/cpp_src/gtests/tests/unit/ft/ft_generic.cc index 069208e6b..f0476ae81 100644 --- a/cpp_src/gtests/tests/unit/ft/ft_generic.cc +++ b/cpp_src/gtests/tests/unit/ft/ft_generic.cc @@ -140,7 +140,7 @@ TEST_P(FTGenericApi, MergeWithSameNSAndSelectFunctions) { for (const auto& query : CreateAllPermutatedQueries("", {"*entity", "somethin*"}, "")) { for (const auto& field : {std::string("ft1"), std::string("ft2")}) { - auto dsl = std::string("@").append(field).append(" ").append(query); + auto dsl = fmt::format("@{} {}", field, query); auto qr{reindexer::Query("nm1").Where("ft3", CondEq, dsl)}; reindexer::QueryResults res; auto mqr{reindexer::Query("nm1").Where("ft3", CondEq, std::move(dsl))}; diff --git a/cpp_src/gtests/tests/unit/ft/ft_incremental_build.cc b/cpp_src/gtests/tests/unit/ft/ft_incremental_build.cc index 10b6c9c42..735bf9df8 100644 --- a/cpp_src/gtests/tests/unit/ft/ft_incremental_build.cc +++ b/cpp_src/gtests/tests/unit/ft/ft_incremental_build.cc @@ -172,8 +172,8 @@ class FTIncrementalBuildApi : public FTApi { return query; } - reindexer::QueryResults SimpleSelect(std::string query) { - auto q = reindexer::Query(GetDefaultNamespace()).Where("ft3", CondEq, std::move(query)).WithRank(); + reindexer::QueryResults SimpleSelect(std::string_view query) { + auto q = reindexer::Query(GetDefaultNamespace()).Where("ft3", CondEq, query).WithRank(); reindexer::QueryResults res; auto err = rt.reindexer->Select(q, res); EXPECT_TRUE(err.ok()) << err.what(); diff --git a/cpp_src/gtests/tests/unit/replication_master_master_test.cc b/cpp_src/gtests/tests/unit/replication_master_master_test.cc index 7b915832f..57866193d 100644 --- a/cpp_src/gtests/tests/unit/replication_master_master_test.cc +++ b/cpp_src/gtests/tests/unit/replication_master_master_test.cc @@ -386,7 +386,11 @@ TEST_F(ReplicationSlaveSlaveApi, MasterSlaveSlaveReload) { TestNamespace1 ns1(master); const int startId = 1000; +#ifdef REINDEX_WITH_ASAN + const int n2 = 4000; +#else // REINDEX_WITH_ASAN const int n2 = 20000; +#endif // REINDEX_WITH_ASAN auto AddThread = [&master, &ns1]() { ns1.AddRow1msSleep(master, startId, n2, ns1.nsName_); }; auto restartServer = [this, &nodes, &kDbPathMaster, &stopRestartServerThread]() { @@ -1115,8 +1119,13 @@ TEST_F(ReplicationSlaveSlaveApi, ConcurrentForceSync) { } // Fill master's data +#ifdef REINDEX_WITH_ASAN + const size_t kRows = 2000; + const size_t kDataBytes = 100; +#else // REINDEX_WITH_ASAN const size_t kRows = 10000; const size_t kDataBytes = 1000; +#endif // REINDEX_WITH_ASAN std::vector testNsList; for (auto& ns : kNsList) { testNsList.emplace_back(nodes[0], ns); diff --git a/cpp_src/gtests/tests/unit/replication_test.cc b/cpp_src/gtests/tests/unit/replication_test.cc index 3a4449ff8..9d7106cb6 100644 --- a/cpp_src/gtests/tests/unit/replication_test.cc +++ b/cpp_src/gtests/tests/unit/replication_test.cc @@ -508,7 +508,11 @@ TEST_F(ReplicationLoadApi, DynamicRoleSwitch) { // Switch master and await sync in each loop iteration for (size_t i = 1; i < 8; i++) { +#ifdef REINDEX_WITH_ASAN + FillData(400); +#else // REINDEX_WITH_ASAN FillData(2000); +#endif // REINDEX_WITH_ASAN WaitSync("some"); WaitSync("some1"); SwitchMaster(i % kDefaultServerCount, {"some", "some1"}); diff --git a/cpp_src/gtests/tests/unit/string_function_test.cc b/cpp_src/gtests/tests/unit/string_function_test.cc index ed60eb25a..307a5b373 100644 --- a/cpp_src/gtests/tests/unit/string_function_test.cc +++ b/cpp_src/gtests/tests/unit/string_function_test.cc @@ -1,4 +1,5 @@ -#if defined(__GNUC__) && (__GNUC__ == 12) && defined(REINDEX_WITH_ASAN) +#if defined(__GNUC__) && ((__GNUC__ == 12) || (__GNUC__ == 13)) && defined(REINDEX_WITH_ASAN) +// regex header is broken in GCC 12.0-13.3 with ASAN #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wmaybe-uninitialized" #include diff --git a/cpp_src/net/cproto/cproto.h b/cpp_src/net/cproto/cproto.h index 6b783d91e..008a3bd3d 100644 --- a/cpp_src/net/cproto/cproto.h +++ b/cpp_src/net/cproto/cproto.h @@ -58,7 +58,7 @@ enum CmdCode : uint16_t { std::string_view CmdName(uint16_t cmd) noexcept; // Maximum number of active queries per client -const uint32_t kMaxConcurentQueries = 256; +const uint32_t kMaxConcurrentQueries = 256; const uint32_t kCprotoMagic = 0xEEDD1132; const uint32_t kCprotoVersion = 0x104; diff --git a/cpp_src/replicator/walrecord.h b/cpp_src/replicator/walrecord.h index a7bcf3595..1f05b9d1f 100644 --- a/cpp_src/replicator/walrecord.h +++ b/cpp_src/replicator/walrecord.h @@ -2,12 +2,12 @@ #include #include -#include #include #include #include "core/keyvalue/p_string.h" #include "estl/chunk.h" #include "estl/h_vector.h" +#include "estl/intrusive_ptr.h" #include "estl/span.h" namespace reindexer { diff --git a/cpp_src/server/rpcserver.cc b/cpp_src/server/rpcserver.cc index 500768980..637af48c8 100644 --- a/cpp_src/server/rpcserver.cc +++ b/cpp_src/server/rpcserver.cc @@ -730,7 +730,7 @@ RPCQrWatcher::Ref RPCServer::createQueryResults(cproto::Context& ctx, RPCQrId& i } } - if rx_unlikely (data->results.size() >= cproto::kMaxConcurentQueries) { + if rx_unlikely (data->results.size() >= cproto::kMaxConcurrentQueries) { for (unsigned idx = 0; idx < data->results.size(); ++idx) { RPCQrId tmpQrId{data->results[idx].main, data->results[idx].uid}; assertrx(tmpQrId.main >= 0); diff --git a/cpp_src/server/vendor/prometheus/impl/check_names.cc b/cpp_src/server/vendor/prometheus/impl/check_names.cc index 9ba1e9e0b..0cac3a0a1 100644 --- a/cpp_src/server/vendor/prometheus/impl/check_names.cc +++ b/cpp_src/server/vendor/prometheus/impl/check_names.cc @@ -4,8 +4,8 @@ #if defined(__GLIBCXX__) && __GLIBCXX__ <= 20150623 #define STD_REGEX_IS_BROKEN #endif -#if defined(__GNUC__) && (__GNUC__ == 12) && (__GNUC_MINOR__ < 4) && defined(REINDEX_WITH_ASAN) -// regex header is broken in GCC 12.0-12.3 with ASAN +#if defined(__GNUC__) && ((__GNUC__ == 12) || (__GNUC__ == 13)) && defined(REINDEX_WITH_ASAN) +// regex header is broken in GCC 12.0-13.3 with ASAN #define STD_REGEX_IS_BROKEN #endif #if defined(_MSC_VER) && _MSC_VER < 1900 @@ -20,7 +20,9 @@ namespace prometheus { bool CheckMetricName(const std::string& name) { // see https://prometheus.io/docs/concepts/data_model/ auto reserved_for_internal_purposes = name.compare(0, 2, "__") == 0; - if (reserved_for_internal_purposes) return false; + if (reserved_for_internal_purposes) { + return false; + } #ifdef STD_REGEX_IS_BROKEN return !name.empty(); #else @@ -32,7 +34,9 @@ bool CheckMetricName(const std::string& name) { bool CheckLabelName(const std::string& name) { // see https://prometheus.io/docs/concepts/data_model/ auto reserved_for_internal_purposes = name.compare(0, 2, "__") == 0; - if (reserved_for_internal_purposes) return false; + if (reserved_for_internal_purposes) { + return false; + } #ifdef STD_REGEX_IS_BROKEN return !name.empty(); #else diff --git a/cpp_src/tools/stringstools.cc b/cpp_src/tools/stringstools.cc index d50f5a953..2fb3c2fca 100644 --- a/cpp_src/tools/stringstools.cc +++ b/cpp_src/tools/stringstools.cc @@ -1,7 +1,6 @@ #include #include #include -#include #include #include @@ -13,7 +12,6 @@ #include "tools/randomgenerator.h" #include "tools/stringstools.h" #include "utf8cpp/utf8.h" -#include "vendor/double-conversion/double-conversion.h" #include "vendor/frozen/unordered_map.h" namespace reindexer { @@ -352,13 +350,38 @@ ComparationResult collateCompare(std::string_view lhs, std::string_ return ComparationResult::Eq; } +static long int strntol(std::string_view str, const char** end, int base) noexcept { + char buf[24]; + long int ret; + const char* beg = str.data(); + auto sz = str.size(); + for (; beg && sz && *beg == ' '; beg++, sz--); + assertrx_dbg(end); + + if (!sz || sz >= sizeof(buf)) { + *end = str.data(); + return 0; + } + + // beg can not be null if sz != 0 + // NOLINTNEXTLINE(clang-analyzer-core.NonNullParamChecker) + std::memcpy(buf, beg, sz); + buf[sz] = '\0'; + ret = std::strtol(buf, const_cast(end), base); + if (ret == LONG_MIN || ret == LONG_MAX) { + return ret; + } + *end = str.data() + (*end - buf); + return ret; +} + template <> ComparationResult collateCompare(std::string_view lhs, std::string_view rhs, const SortingPrioritiesTable&) noexcept { - char* posl = nullptr; - char* posr = nullptr; + const char* posl = nullptr; + const char* posr = nullptr; - int numl = strtol(lhs.data(), &posl, 10); - int numr = strtol(rhs.data(), &posr, 10); + int numl = strntol(lhs, &posl, 10); + int numr = strntol(rhs, &posr, 10); if (numl == numr) { auto minlen = std::min(lhs.size() - (posl - lhs.data()), rhs.size() - (posr - rhs.data())); diff --git a/cpp_src/tools/varint.h b/cpp_src/tools/varint.h index 1fbddd201..65314aaf2 100644 --- a/cpp_src/tools/varint.h +++ b/cpp_src/tools/varint.h @@ -260,5 +260,5 @@ inline unsigned scan_varint(unsigned len, const uint8_t* data) noexcept { #ifndef _MSC_VER #pragma GCC diagnostic pop #else -#pragma warning(push) +#pragma warning(pop) #endif