Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make snapshot load faster #222

Merged
merged 3 commits into from
Mar 29, 2024

Conversation

lzydmxy
Copy link
Contributor

@lzydmxy lzydmxy commented Mar 25, 2024

Which issues of this PR fixes:

This PR try to fix #221
For nodes count 58680021, snapshot size 12GB

Before(sec) After(sec)
Parse from disk 90 70
Build children path 90 9

Change log:

Optimizations:

  1. Reduced one buffer copy and destruction.
  2. Storage edges in bucketed based on the parent node's bucket number, and built childrenSet in parallel.
  3. Reserved space for childrenSet in advance.

@@ -297,6 +301,9 @@ class KeeperStore
/// Build path children after load data from snapshot
void buildPathChildren(bool from_zk_snapshot = false);

// Build childrenset for the node in specified block after load data from snapshot.
void buildBlockChildren(const std::vector<BlocksEdges> & all_objects_edges, UInt32 block_idx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from_zk_snapshot is omited

src/Service/KeeperStore.h Outdated Show resolved Hide resolved
src/Service/KeeperStore.h Outdated Show resolved Hide resolved
@JackyWoo
Copy link
Contributor

@lzydmxy Nice optimization! Please describe your solution.

@JackyWoo
Copy link
Contributor

JackyWoo commented Mar 26, 2024

Is there any optimization for parsing data from disk? For I found it is faster in your benchmark.

@lzydmxy
Copy link
Contributor Author

lzydmxy commented Mar 26, 2024

@lzydmxy Nice optimization! Please describe your solution.
Here are three optimization points:
Reduced one buffer copy and destruction.
Storage edges in bucketed based on the parent node's bucket number, and built childrenSet in parallel.
Reserved space for childrenSet in advance.

@JackyWoo JackyWoo added the performance Performance promotion label Mar 26, 2024
@JackyWoo JackyWoo added this to the Release v2.0.5 milestone Mar 26, 2024
}
catch (Exception & e)
{
LOG_ERROR(log, "parseObject error {}, {}", it->second, getExceptionMessage(e, true));
throw;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please throw Exception and add an error code

@lzydmxy
Copy link
Contributor Author

lzydmxy commented Mar 27, 2024

@JackyWoo The test failed for apple clang, should we continue support it ?

Run export CC=`which clang` CXX=`which clang++` && cmake -G Ninja -B ./build -DCMAKE_BUILD_TYPE=Release
-- CMAKE_TOOLCHAIN_FILE - 
-- The C compiler identification is AppleClang 15.0.0.15000040
-- The CXX compiler identification is AppleClang 15.0.0.15000040

@JackyWoo
Copy link
Contributor

We should not use Apple clang. Let us update the CI env.

@JackyWoo JackyWoo merged commit 884a2ed into JDRaftKeeper:master Mar 29, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance promotion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accelerate snapshot loading
2 participants