Implement Adaptive-IBLT#22
Open
Chenxy517 wants to merge 24 commits into
Open
Conversation
trachten
reviewed
Jun 27, 2025
Contributor
trachten
left a comment
trachten
left a comment
There was a problem hiding this comment.
Please consider my comments and resubmit.
| ${SYNC_BENCH_INC}/FromFileGen.h | ||
| ) | ||
|
|
||
| if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux") |
Contributor
There was a problem hiding this comment.
There are many other possible systems (e.g., *BSD,iOS, ...) ... it may be more robust to simply check for the existance of a known library or include in a given directory.
| void commSend(const Cuckoo &cf); | ||
|
|
||
| /** | ||
| * Sends list of size_t. |
Contributor
There was a problem hiding this comment.
size_t is a type ... why is it being referred to as a parameter?
|
|
||
| namespace std { | ||
| template <> | ||
| struct hash<NTL::ZZ> { |
Contributor
There was a problem hiding this comment.
Why is this in the std namespace? If it's a utility, it should be put in Auxiliary.h. If it is specific to Adapatibe_IBLT, then, perhaps, it should be a subclass.
|
|
||
| class Adaptive_IBLT : public IBLT { | ||
| public: | ||
| Adaptive_IBLT(long numHashes, long numHashCheck, size_t expectedNumEntries, size_t valueSize); |
Contributor
There was a problem hiding this comment.
This needs javadoc documentation.
| public: | ||
| Adaptive_IBLT(long numHashes, long numHashCheck, size_t expectedNumEntries, size_t valueSize); | ||
|
|
||
| // List entries and record the peeled cell indices |
Contributor
There was a problem hiding this comment.
please add proper javadoc, including parameter annotations
| peeledKeys.insert(SMOKeys.begin(), SMOKeys.end()); | ||
| mySyncStats.timerEnd(SyncStats::COMP_TIME); | ||
|
|
||
| // For test |
Contributor
There was a problem hiding this comment.
You can put this through the Logger infrastructure.
| .setValueSize(elementSize) | ||
| .build(); | ||
|
|
||
| std::cout << "[Server]: Current Size: " << currentExpected << std::endl; |
| } | ||
| } | ||
|
|
||
| bool IBLTSync_Adaptive_PartialDecode::SyncServer(const shared_ptr<Communicant>& commSync, |
Contributor
There was a problem hiding this comment.
Again ... fairly long procedure; see above.
| } | ||
| std::cout << "Elements added." << std::endl; | ||
|
|
||
| if (fork()) { |
Contributor
There was a problem hiding this comment.
You should use the testing infrastructure for Gensync (which I plan to overhaul soon).
| void testGetStrings(); | ||
| }; | ||
|
|
||
| #endif //GENSYNC_IBLTSYNC_ADAPTIVETEST_H |
Contributor
There was a problem hiding this comment.
What about your updated IBLT tests? Listing tests?
Contributor
Author
|
Well received with thanks! I will address them and resubmit together
with the adaptiveIBLT with partial decoding.
…On Fri, Jun 27, 2025 at 3:14 PM Ari Trachtenberg ***@***.***> wrote:
***@***.**** commented on this pull request.
Please consider my comments and resubmit.
------------------------------
In CMakeLists.txt
<#22 (comment)>:
> @@ -138,6 +142,25 @@ set(HEADERS
${SYNC_BENCH_INC}/FromFileGen.h
)
+if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
There are many other possible systems (e.g., *BSD,iOS, ...) ... it may be
more robust to simply check for the existance of a known library or include
in a given directory.
------------------------------
In include/GenSync/Communicants/Communicant.h
<#22 (comment)>:
> @@ -263,6 +263,12 @@ class Communicant {
*/
void commSend(const Cuckoo &cf);
+ /**
+ * Sends list of size_t.
size_t is a type ... why is it being referred to as a parameter?
------------------------------
In include/GenSync/Syncs/Adaptive_IBLT.h
<#22 (comment)>:
> @@ -0,0 +1,36 @@
+//
+// Created by ChenXingyu on 6/10/25.
+//
+
+#ifndef GENSYNC_ADAPTIVE_IBLT_H
+#define GENSYNC_ADAPTIVE_IBLT_H
+
+#include <GenSync/Syncs/IBLT.h>
+#include <unordered_set>
+
+namespace std {
+ template <>
+ struct hash<NTL::ZZ> {
Why is this in the std namespace? If it's a utility, it should be put in
Auxiliary.h. If it is specific to Adapatibe_IBLT, then, perhaps, it should
be a subclass.
------------------------------
In include/GenSync/Syncs/Adaptive_IBLT.h
<#22 (comment)>:
> +
+#include <GenSync/Syncs/IBLT.h>
+#include <unordered_set>
+
+namespace std {
+ template <>
+ struct hash<NTL::ZZ> {
+ std::size_t operator()(const NTL::ZZ& z) const {
+ return std::hash<std::string>()(zzToString(z));
+ }
+ };
+}
+
+class Adaptive_IBLT : public IBLT {
+public:
+ Adaptive_IBLT(long numHashes, long numHashCheck, size_t expectedNumEntries, size_t valueSize);
This needs javadoc documentation.
------------------------------
In include/GenSync/Syncs/Adaptive_IBLT.h
<#22 (comment)>:
> +#include <unordered_set>
+
+namespace std {
+ template <>
+ struct hash<NTL::ZZ> {
+ std::size_t operator()(const NTL::ZZ& z) const {
+ return std::hash<std::string>()(zzToString(z));
+ }
+ };
+}
+
+class Adaptive_IBLT : public IBLT {
+public:
+ Adaptive_IBLT(long numHashes, long numHashCheck, size_t expectedNumEntries, size_t valueSize);
+
+ // List entries and record the peeled cell indices
please add proper javadoc, including parameter annotations
------------------------------
In include/GenSync/Syncs/Adaptive_IBLT.h
<#22 (comment)>:
> + std::size_t operator()(const NTL::ZZ& z) const {
+ return std::hash<std::string>()(zzToString(z));
+ }
+ };
+}
+
+class Adaptive_IBLT : public IBLT {
+public:
+ Adaptive_IBLT(long numHashes, long numHashCheck, size_t expectedNumEntries, size_t valueSize);
+
+ // List entries and record the peeled cell indices
+ bool listEntries(std::vector<std::pair<NTL::ZZ, NTL::ZZ>> &positive,
+ std::vector<std::pair<NTL::ZZ, NTL::ZZ>> &negative,
+ std::vector<size_t> &peeledIndicesOut);
+
+ // Return elements reconstructed from cell indices
do both these methods need to be public?
------------------------------
In include/GenSync/Syncs/IBLT.h
<#22 (comment)>:
> @@ -139,6 +139,16 @@ class IBLT : public GenIBLT
*/
IBLT(long numHashes, long numHashCheck, size_t expectedNumEntries, size_t valueSize);
+
+ bool listEntriesandKeys(vector<pair<ZZ, ZZ>> &positive,
+ vector<pair<ZZ, ZZ>> &negative,
+ vector<ZZ> &OMSKeys,
+ vector<ZZ> &SMOKeys);
+
+ bool partialPeelFromCells(
javadoc
------------------------------
In include/GenSync/Syncs/IBLTSync_Adaptive.h
<#22 (comment)>:
> @@ -0,0 +1,63 @@
+/* This code is part of the GenSync project developed at Boston University. Please see the README for use and references. */
+
+/*
+ * The IBLTSync_Adaptive method syncs with another IBLTSync_Adaptive method by sending an IBLT containing
use ***@***.*** <https://github.com/link> <>} in case the class name is
refactored later.
------------------------------
In include/GenSync/Syncs/IBLTSync_Adaptive.h
<#22 (comment)>:
> @@ -0,0 +1,63 @@
+/* This code is part of the GenSync project developed at Boston University. Please see the README for use and references. */
+
+/*
+ * The IBLTSync_Adaptive method syncs with another IBLTSync_Adaptive method by sending an IBLT containing
+ * its set. Upon receiving this IBLT, the server performs a subtract operation on both IBLTs
+ * and uses the resulting IBLT to calculate the symmetric set difference. These differences
+ * are then sent back to the client. If the decoding process failed, we reconstruct an IBLT with doubled size and re-run the synchronization
reconstruct -> construct
------------------------------
In include/GenSync/Syncs/IBLTSync_Adaptive.h
<#22 (comment)>:
> @@ -0,0 +1,63 @@
+/* This code is part of the GenSync project developed at Boston University. Please see the README for use and references. */
+
+/*
+ * The IBLTSync_Adaptive method syncs with another IBLTSync_Adaptive method by sending an IBLT containing
+ * its set. Upon receiving this IBLT, the server performs a subtract operation on both IBLTs
+ * and uses the resulting IBLT to calculate the symmetric set difference. These differences
+ * are then sent back to the client. If the decoding process failed, we reconstruct an IBLT with doubled size and re-run the synchronization
+ * There is a small probability that most, but not all, of the differences will be uncovered as a result
I would just say that there is an adjustable probability that the sync
will fail to recover all differences.
------------------------------
In include/GenSync/Syncs/IBLTSync_Adaptive.h
<#22 (comment)>:
> +#ifndef GENSYNC_IBLTSYNC_ADAPTIVE_H
+#define GENSYNC_IBLTSYNC_ADAPTIVE_H
+
+#include <GenSync/Aux/SyncMethod.h>
+#include <GenSync/Syncs/IBLT.h>
+
+/**
+ * IBLTSync_Adaptive class dynamically grows the IBLT if reconciliation fails.
+ */
+class IBLTSync_Adaptive : public SyncMethod
+{
+public:
+ /*
+ * Constructor.
+ * @param initExpectedEntries The initial value of expected number of elements being stored
+ * @param eltSize The size of elements being stored
This is not clear
------------------------------
In include/GenSync/Syncs/IBLTSync_Adaptive.h
<#22 (comment)>:
> +
+#ifndef GENSYNC_IBLTSYNC_ADAPTIVE_H
+#define GENSYNC_IBLTSYNC_ADAPTIVE_H
+
+#include <GenSync/Aux/SyncMethod.h>
+#include <GenSync/Syncs/IBLT.h>
+
+/**
+ * IBLTSync_Adaptive class dynamically grows the IBLT if reconciliation fails.
+ */
+class IBLTSync_Adaptive : public SyncMethod
+{
+public:
+ /*
+ * Constructor.
+ * @param initExpectedEntries The initial value of expected number of elements being stored
The initial *guess* of ...
------------------------------
In include/GenSync/Syncs/IBLTSync_Adaptive_PartialDecode.h
<#22 (comment)>:
> @@ -0,0 +1,54 @@
+//
+// Created by ChenXingyu on 6/5/25.
+//
+
+#ifndef GENSYNC_IBLTSYNC_ADAPTIVE_PARTIALDECODE_H
+#define GENSYNC_IBLTSYNC_ADAPTIVE_PARTIALDECODE_H
+
+#include <GenSync/Aux/SyncMethod.h>
+#include <GenSync/Syncs/IBLT.h>
+
+/**
+ * IBLTSync_Adaptive class dynamically grows the IBLT if reconciliation fails.
The distinction between IBLTSync_Adaptive and this class is not clear.
------------------------------
In src/Communicants/Communicant.cpp
<#22 (comment)>:
> @@ -346,6 +345,14 @@ void Communicant::commSend(const ZZ& num, Nullable<size_t> size) {
}
+void Communicant::commSend(const std::vector<ZZ> &vec) {
Doesn't NTL also have a vec_ZZ class (or the like)?
------------------------------
In src/Syncs/Adaptive_IBLT.cpp
<#22 (comment)>:
> @@ -0,0 +1,49 @@
+//
+// Created by ChenXingyu on 6/10/25.
+//
+#include <GenSync/Syncs/Adaptive_IBLT.h>
+#include <GenSync/Aux/Logger.h>
+
+Adaptive_IBLT::Adaptive_IBLT(long numHashes, long numHashCheck, size_t expectedNumEntries, size_t valueSize)
+ : IBLT(numHashes, numHashCheck, expectedNumEntries, valueSize) {}
+
+bool Adaptive_IBLT::listEntries(std::vector<std::pair<NTL::ZZ, NTL::ZZ>> &positive,
It would be good to comment the internal workings here ... it won't be
clear to you in a few months.
------------------------------
In src/Syncs/Adaptive_IBLT.cpp
<#22 (comment)>:
> + do {
+ nErased = 0;
+ for (size_t i = 0; i < hashTable.size(); ++i) {
+ auto& entry = hashTable[i];
+ if (entry.isPure(numHashCheck)) {
+ if (entry.count == 1) {
+ positive.emplace_back(entry.keySum, entry.valueSum);
+ } else {
+ negative.emplace_back(entry.keySum, entry.valueSum);
+ }
+ this->_insert(-entry.count, entry.keySum, entry.valueSum);
+ peeledIndices.push_back(i);
+ ++nErased;
+ }
+ }
+ } while (nErased > 0);
How does this avoid an infinite loop?
------------------------------
In src/Syncs/IBLT.cpp
<#22 (comment)>:
> + nErased = 0;
+ for (size_t i = 0; i < hashTable.size(); ++i) {
+ auto& entry = hashTable[i];
+ if (entry.isPure(numHashCheck)) {
+ if (entry.count == 1) {
+ positive.emplace_back(entry.keySum, entry.valueSum);
+ OMSKeys.push_back(entry.keySum); // record key of positive
+ } else {
+ negative.emplace_back(entry.keySum, entry.valueSum);
+ SMOKeys.push_back(entry.keySum); // record key of negative
+ }
+ this->_insert(-entry.count, entry.keySum, entry.valueSum);
+ ++nErased;
+ }
+ }
+ } while (nErased > 0);
Same questions as before ... and, this looks very similar to the code
above - can it be combined into one helper function?
------------------------------
In src/Syncs/IBLTSync_Adaptive.cpp
<#22 (comment)>:
> @@ -0,0 +1,254 @@
+//
+// Created by Xingyu Chen on 4/27/25.
+//
+
+#include <GenSync/Syncs/IBLTSync_Adaptive.h>
+#include <GenSync/Aux/Exceptions.h>
+
+IBLTSync_Adaptive::IBLTSync_Adaptive(size_t initExpected, size_t eltSize) {
+ initExpNumElems = initExpected;
+ elementSize = eltSize;
+
+ myIBLT = IBLT::Builder().
+ setNumHashes(4).
I suggest making these default values constants in the .h file.
------------------------------
In src/Syncs/IBLTSync_Adaptive.cpp
<#22 (comment)>:
> + size_t currentExpected = initExpNumElems;
+
+ // call parent method for bookkeeping
+ SyncMethod::SyncClient(commSync, selfMinusOther, otherMinusSelf);
+
+ // connect to server
+ mySyncStats.timerStart(SyncStats::IDLE_TIME);
+ commSync->commConnect();
+ mySyncStats.timerEnd(SyncStats::IDLE_TIME);
+
+ // keep running until peeling succeed
+ while (true) {
+ commSync->commSend(static_cast<int>(currentExpected));
+
+ // construct new IBLT with updated size
+ myIBLT = IBLT::Builder()
This is the same code as before ... it should probably be relegated to a
helper function.
------------------------------
In src/Syncs/IBLTSync_Adaptive.cpp
<#22 (comment)>:
> + }
+
+ // ensure that the IBLT size and eltSize equal those of the server otherwise fail and don't continue
+ mySyncStats.timerStart(SyncStats::COMM_TIME);
+ if (!commSync->establishIBLTSend(myIBLT.size(), myIBLT.eltSize(), false)) {
+ Logger::error_and_quit("IBLT parameter mismatch during SyncClient.");
+ mySyncStats.timerEnd(SyncStats::COMM_TIME);
+ mySyncStats.increment(SyncStats::XMIT,commSync->getXmitBytes());
+ mySyncStats.increment(SyncStats::RECV,commSync->getRecvBytes());
+ return false;
+ }
+ commSync->commSend(myIBLT, true);
+ bool success = commSync->commRecv_int();
+ mySyncStats.timerEnd(SyncStats::COMM_TIME);
+
+// mySyncStats.timerStart(SyncStats::COMM_TIME);
Why is this here?
------------------------------
In src/Syncs/IBLTSync_Adaptive.cpp
<#22 (comment)>:
> + list<shared_ptr<DataObject>> newOMS = commSync->commRecv_DataObject_List();
+ list<shared_ptr<DataObject>> newSMO = commSync->commRecv_DataObject_List();
+ mySyncStats.timerEnd(SyncStats::COMM_TIME);
+
+ mySyncStats.timerStart(SyncStats::COMP_TIME);
+ otherMinusSelf.insert(otherMinusSelf.end(), newOMS.begin(), newOMS.end());
+ selfMinusOther.insert(selfMinusOther.end(), newSMO.begin(), newSMO.end());
+ mySyncStats.timerEnd(SyncStats::COMP_TIME);
+
+ mySyncStats.increment(SyncStats::XMIT, commSync->getXmitBytes());
+ mySyncStats.increment(SyncStats::RECV, commSync->getRecvBytes());
+ return true;
+ }
+ }
+}
+ // insert the decoded elements and record Stats
Is this needed in the code?
------------------------------
In src/Syncs/IBLTSync_Adaptive.cpp
<#22 (comment)>:
> + size_t currentExpected = initExpNumElems;
+
+
+ // call parent method for bookkeeping
+ SyncMethod::SyncServer(commSync, selfMinusOther, otherMinusSelf);
+
+ // listen for client
+ mySyncStats.timerStart(SyncStats::IDLE_TIME);
+ commSync->commListen();
+ mySyncStats.timerEnd(SyncStats::IDLE_TIME);
+
+ while (true) {
+ currentExpected = static_cast<size_t>(commSync->commRecv_int());
+
+ // construct new IBLT with updated size
+ myIBLT = IBLT::Builder()
see comment before ... you want these in one common procedure so that if
you make changes one place, they get reflected elsewhere.
------------------------------
In src/Syncs/IBLTSync_Adaptive.cpp
<#22 (comment)>:
> + mySyncStats.timerStart(SyncStats::IDLE_TIME);
+ commSync->commListen();
+ mySyncStats.timerEnd(SyncStats::IDLE_TIME);
+
+ while (true) {
+ currentExpected = static_cast<size_t>(commSync->commRecv_int());
+
+ // construct new IBLT with updated size
+ myIBLT = IBLT::Builder()
+ .setNumHashes(4)
+ .setNumHashCheck(11)
+ .setExpectedNumEntries(currentExpected)
+ .setValueSize(elementSize)
+ .build();
+
+ std::cout << "[Server]: Current Size: " << currentExpected << std::endl;
This should be printed through the Logger.
------------------------------
In src/Syncs/IBLTSync_Adaptive.cpp
<#22 (comment)>:
> +// mySyncStats.timerStart(SyncStats::COMP_TIME);
+// otherMinusSelf.insert(otherMinusSelf.end(), newOMS.begin(), newOMS.end());
+// selfMinusOther.insert(selfMinusOther.end(), newSMO.begin(), newSMO.end());
+// mySyncStats.timerEnd(SyncStats::COMP_TIME);
+//
+// mySyncStats.increment(SyncStats::XMIT, commSync->getXmitBytes());
+// mySyncStats.increment(SyncStats::RECV, commSync->getRecvBytes());
+//
+// return true;
+// } else {
+// Logger::gLog(Logger::METHOD_DETAILS, "Sync failed. Doubling IBLT size to " + toStr(currentExpected * 2));
+// currentExpected *= 2;
+// // commSync->commClose();
+// }
+
+bool IBLTSync_Adaptive::SyncServer(const shared_ptr<Communicant>& commSync,
This is a big procedure ... you should have general guideline comments
within, or split it up into smaller functions.
------------------------------
In src/Syncs/IBLTSync_Adaptive_PartialDecode.cpp
<#22 (comment)>:
> +// Created by ChenXingyu on 6/5/25.
+//
+
+#include <GenSync/Syncs/IBLT.h>
+#include <GenSync/Syncs/Adaptive_IBLT.h>
+#include <GenSync/Syncs/IBLTSync_Adaptive_PartialDecode.h>
+#include <GenSync/Aux/Exceptions.h>
+#include <NTL/ZZ.h>
+#include <functional>
+#include <unordered_set>
+
+IBLTSync_Adaptive_PartialDecode::IBLTSync_Adaptive_PartialDecode(size_t initExpected, size_t eltSize) {
+ initExpNumElems = initExpected;
+ elementSize = eltSize;
+
+ myIBLT = IBLT::Builder().
See above
------------------------------
In src/Syncs/IBLTSync_Adaptive_PartialDecode.cpp
<#22 (comment)>:
> + unordered_set<ZZ> peeledKeys;
+
+ // call parent method for bookkeeping
+ SyncMethod::SyncClient(commSync, selfMinusOther, otherMinusSelf);
+
+ // connect to server
+ mySyncStats.timerStart(SyncStats::IDLE_TIME);
+ commSync->commConnect();
+ mySyncStats.timerEnd(SyncStats::IDLE_TIME);
+
+ // keep running until peeling succeed
+ while (true) {
+ commSync->commSend(static_cast<int>(currentExpected));
+
+ // construct new IBLT with updated size
+ myIBLT = IBLT::Builder()
see above
------------------------------
In src/Syncs/IBLTSync_Adaptive_PartialDecode.cpp
<#22 (comment)>:
> + commSync->commConnect();
+ mySyncStats.timerEnd(SyncStats::IDLE_TIME);
+
+ // keep running until peeling succeed
+ while (true) {
+ commSync->commSend(static_cast<int>(currentExpected));
+
+ // construct new IBLT with updated size
+ myIBLT = IBLT::Builder()
+ .setNumHashes(4)
+ .setNumHashCheck(11)
+ .setExpectedNumEntries(currentExpected)
+ .setValueSize(elementSize)
+ .build();
+
+// for (auto iter = SyncMethod::beginElements(); iter != SyncMethod::endElements(); iter++) {
is this needed?
------------------------------
In src/Syncs/IBLTSync_Adaptive_PartialDecode.cpp
<#22 (comment)>:
> + if (peeledKeys.find(key) != peeledKeys.end()) {
+ std::cout << "[Server] Skip inserting already peeled key: " << key << std::endl;
+ continue;
+ }
+ myIBLT.insert(key, key);
+ }
+
+ mySyncStats.timerStart(SyncStats::COMP_TIME);
+ vector<pair<ZZ, ZZ>> positive, negative;
+ vector<ZZ> OMSKeys, SMOKeys;
+ clientIBLT -= myIBLT;
+ bool peelSuccess = clientIBLT.listEntriesandKeys(positive, negative, OMSKeys, SMOKeys);
+ peeledKeys.insert(SMOKeys.begin(), SMOKeys.end());
+ mySyncStats.timerEnd(SyncStats::COMP_TIME);
+
+ // For test
You can put this through the Logger infrastructure.
------------------------------
In src/Syncs/IBLTSync_Adaptive_PartialDecode.cpp
<#22 (comment)>:
> + mySyncStats.timerStart(SyncStats::IDLE_TIME);
+ commSync->commListen();
+ mySyncStats.timerEnd(SyncStats::IDLE_TIME);
+
+ while (true) {
+ currentExpected = static_cast<size_t>(commSync->commRecv_int());
+
+ // construct new IBLT with updated size
+ myIBLT = Adaptive_IBLT::Builder()
+ .setNumHashes(4)
+ .setNumHashCheck(11)
+ .setExpectedNumEntries(currentExpected)
+ .setValueSize(elementSize)
+ .build();
+
+ std::cout << "[Server]: Current Size: " << currentExpected << std::endl;
Logger
------------------------------
In src/Syncs/IBLTSync_Adaptive_PartialDecode.cpp
<#22 (comment)>:
> + auto dataPtr = make_shared<DataObject>(key);
+ newOMS.push_back(dataPtr);
+ }
+
+ otherMinusSelf.insert(otherMinusSelf.end(), newOMS.begin(), newOMS.end());
+ selfMinusOther.insert(selfMinusOther.end(), newSMO.begin(), newSMO.end());
+ mySyncStats.timerEnd(SyncStats::COMP_TIME);
+
+ mySyncStats.increment(SyncStats::XMIT, commSync->getXmitBytes());
+ mySyncStats.increment(SyncStats::RECV, commSync->getRecvBytes());
+ return true;
+ }
+ }
+}
+
+bool IBLTSync_Adaptive_PartialDecode::SyncServer(const shared_ptr<Communicant>& commSync,
Again ... fairly long procedure; see above.
------------------------------
In src/test_adaptive.cpp
<#22 (comment)>:
> +
+ for (int i = 0; i < 1000; ++i) {
+ string elem = "e" + to_string(i);
+ if (i < 5) {
+ // insert [0,19] to both sides
+ host1.addElem(make_shared<DataObject>(elem));
+ host2.addElem(make_shared<DataObject>(elem));
+ } else {
+ // insert [20,29] to host1, [30,39] to host2
+ host1.addElem(make_shared<DataObject>(elem));
+ host2.addElem(make_shared<DataObject>("e" + to_string(i+995)));
+ }
+ }
+ std::cout << "Elements added." << std::endl;
+
+ if (fork()) {
You should use the testing infrastructure for Gensync (which I plan to
overhaul soon).
------------------------------
In tests/unit/IBLTSync_AdaptiveTest.h
<#22 (comment)>:
> + * Test reconciliation of large sets using IBLTSync_Adaptive
+ */
+ void IBLTSync_AdaptiveLargeSetReconcileTest();
+
+ /**
+ * Test adding and deleting elements
+ */
+ void testAddDelElem();
+
+ /**
+ * Test that printElem() and getName() return some nonempty string
+ */
+ void testGetStrings();
+};
+
+#endif //GENSYNC_IBLTSYNC_ADAPTIVETEST_H
What about your updated IBLT tests? Listing tests?
—
Reply to this email directly, view it on GitHub
<#22 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AUD4FBTR25DQQG7CMDUNHQL3FWJZJAVCNFSM6AAAAAB6BPWPOCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDSNRXGYZDANRQGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
GregoryFan
reviewed
Jul 29, 2025
| ZZ key = (**iter).to_ZZ(); | ||
|
|
||
| if (peeledKeys.find(key) != peeledKeys.end()) { | ||
| std::cout << "[Client] Skip inserting already peeled key: " << key << std::endl; |
Contributor
|
There's not a lot I can add on as the major points have already been said, but other than those looks fine apart from a few places where |
trachten
requested changes
Jul 31, 2025
| void Communicant::commSend(const vec_ZZ& vec) { | ||
| Logger::gLog(Logger::COMM, "... attempting to send: vec_ZZ " + toStr(vec)); | ||
|
|
||
| const ZZ base = power_ZZ(2, 256) + 1; |
Contributor
There was a problem hiding this comment.
why 2^256? What if individual ZZ's are larger? Please add a test that considers very large ZZ to catch this.
trachten
requested changes
Aug 11, 2025
Contributor
trachten
left a comment
trachten
left a comment
There was a problem hiding this comment.
Looks better ... still some minor corrections are needed.
| /opt/local/include | ||
| /opt/homebrew/include | ||
| ) | ||
| if(NTL_INCLUDE_PATH MATCHES "/usr/local/include") |
Contributor
There was a problem hiding this comment.
Isn't there a cleaner way of getting the parent directory?
|
|
||
| /** | ||
| * Sends a list of ZZ over the line. | ||
| * @param The list of ZZ to send. |
Contributor
There was a problem hiding this comment.
technically, a vector of ZZ to send?
| public: | ||
| /** | ||
| * Constructor. | ||
| * @param initExpected The initial guess of expected number of elements being stored |
Contributor
There was a problem hiding this comment.
What does "number of elements beingstored" mean?
| * its set. Upon receiving this IBLT, the server performs a subtract operation on both IBLTs | ||
| * and uses the resulting IBLT to calculate the symmetric set difference. These differences | ||
| * are then sent back to the client. If the decoding process failed, we construct an new IBLT. | ||
| * The new IBLT is built with doubled size minus the number of peeled elements and we do not insert peeled elements. |
Contributor
There was a problem hiding this comment.
Maybe a note that doubling in size is the optimal strategy with respect to overhead over the "clairvoyant" IBLT?
| size_t elementSize; | ||
|
|
||
| // Provides a hash function for NTL::ZZ by converting the value to a string and hashing that. | ||
| struct HashZZ { |
| ZZ divisor, remainder; | ||
| DivRem(divisor, remainder, received, base); | ||
|
|
||
| append(result, to_ZZ(remainder-1)); // subtract back the 1 that was added when sent |
Contributor
There was a problem hiding this comment.
Again, I'm not sure why you need the -1.
| currentExpected = static_cast<size_t>(commSync->commRecv_int()); | ||
| mySyncStats.timerEnd(SyncStats::COMM_TIME); | ||
|
|
||
| // construct new IBLT with updated size |
Contributor
There was a problem hiding this comment.
This could be a helper function.
| myIBLT.insert((**iter).to_ZZ(), (**iter).to_ZZ()); | ||
| } | ||
|
|
||
| // Attempt to peel the elements |
Contributor
There was a problem hiding this comment.
This could also be a helper function.
| mySyncStats.timerEnd(SyncStats::IDLE_TIME); | ||
|
|
||
| // keep running until peeling succeed | ||
| while (true) { |
Contributor
There was a problem hiding this comment.
A bit dangerous ... do you want a timeout in case of stalling?
| if (peeledKeys.find(key) != peeledKeys.end()) { | ||
| continue; | ||
| } | ||
| myIBLT.insert(key, key); |
Contributor
There was a problem hiding this comment.
It seems wasteful to use a value that is identical to the key.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.