Skip to content

Commit ee1d92b

Browse files
got3nksmrjimenez
got3nks
authored and
mrjimenez
committed
EC skip-unchanged (5/5): extend partial-update protocol to amuleweb
amuleweb drives its INC_UPDATE polling through `EC_OP_GET_SHARED_FILES` + `EC_OP_GET_DLOAD_QUEUE` with `EC_DETAIL_UPDATE` rather than the `EC_OP_GET_UPDATE` path amulegui uses. Commit (4/5) wired the partial- update protocol only into `Get_EC_Response_GetUpdate`, so amuleweb kept iterating all files every cycle even on huge libraries. Extend the same skip-unchanged + `EC_TAG_FILE_REMOVED` logic into `Get_EC_Response_GetSharedFiles` and `Get_EC_Response_GetDownloadQueue`, guarded on the same `m_partialUpdateActive` flag and only kicking in for `EC_DETAIL_UPDATE` requests (so amulecmd's `show shared` and other `EC_DETAIL_FULL` callers are untouched). Per-path state because the three handlers fire on independent cadences: * `m_lastEcGenSeen` + `m_lastSentFileIds` — amulegui's `EC_OP_GET_UPDATE` * `m_lastEcGenSeenShared` + `m_lastSentSharedFileIds` — amuleweb's `EC_OP_GET_SHARED_FILES` * `m_lastEcGenSeenPart` + `m_lastSentPartFileIds` — amuleweb's `EC_OP_GET_DLOAD_QUEUE` amuleweb's `UpdatableItemsContainer::ProcessUpdate` learns the same two-path behaviour: when the negotiated capability is active, drain `EC_TAG_FILE_REMOVED` markers into a buffered removal set and run a single sweep over the live list only when removals exist; otherwise the legacy "missing == deleted" loop stays correct (server still emits diff-empty alive-marker tags via the existing encoder path). `CaMuleExternalConnector::IsServerPartialUpdateActive()` exposes `CRemoteConnect::ServerSupportsPartialUpdate()` to the container. Compat matrix unchanged from (4/5): the negotiation is symmetric, so old amuleweb + new amuled and new amuleweb + old amuled both fall back to the legacy path automatically.
1 parent 93a0676 commit ee1d92b

3 files changed

Lines changed: 161 additions & 23 deletions

File tree

‎src/ExternalConn.cpp‎

Lines changed: 111 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -234,11 +234,18 @@ class CECServerSocket : public CECMuleSocket
234234
// queue. See WriteDoneAndQueueEmpty for the full reasoning.
235235
int m_notification_dispatch_depth;
236236

237-
// EC INC_UPDATE skip-unchanged state. `m_lastEcGenSeen` is the highest
238-
// `CKnownFile::s_globalEcGen` value already reflected in this
239-
// connection's `m_obj_tagmap` — files with a smaller `m_ecGen` did
240-
// not change since the last response and can be skipped this cycle.
237+
// EC INC_UPDATE skip-unchanged state. `m_lastEcGenSeen*` values are
238+
// the highest `CKnownFile::s_globalEcGen` already reflected in the
239+
// client's view *for that particular request path* — files with a
240+
// smaller `m_ecGen` did not change since the last response of that
241+
// path and can be skipped this cycle. Three independent counters
242+
// because the request paths interleave on different schedules:
243+
// * `m_lastEcGenSeen` — `EC_OP_GET_UPDATE` (amulegui)
244+
// * `m_lastEcGenSeenShared` — `EC_OP_GET_SHARED_FILES` (amuleweb)
245+
// * `m_lastEcGenSeenPart` — `EC_OP_GET_DLOAD_QUEUE` (amuleweb)
241246
uint64 m_lastEcGenSeen;
247+
uint64 m_lastEcGenSeenShared;
248+
uint64 m_lastEcGenSeenPart;
242249

243250
// Client opted in to partial-update protocol at auth time (advertised
244251
// `EC_TAG_CAN_PARTIAL_UPDATE`). When set, `Get_EC_Response_GetUpdate`
@@ -249,10 +256,16 @@ class CECServerSocket : public CECMuleSocket
249256
// unchanged files so old clients (which infer deletion from absence)
250257
// keep working unchanged — see `Get_EC_Response_GetUpdate`.
251258
bool m_partialUpdateActive;
252-
// Set of file ECIDs sent in the previous INC_UPDATE response. Diffed
253-
// against the current encoder snapshot each cycle to compute the
254-
// removal list emitted to partial-update-capable clients.
259+
// Set of file ECIDs sent in the previous response for each EC
260+
// request path. Diffed against the current snapshot to compute the
261+
// removal list emitted to partial-update-capable clients. Tracked
262+
// per-path because amulegui uses `EC_OP_GET_UPDATE` (mixed shared +
263+
// partfile, served by `Get_EC_Response_GetUpdate`) while amuleweb
264+
// drives two separate INC_UPDATE streams via `EC_OP_GET_SHARED_FILES`
265+
// and `EC_OP_GET_DLOAD_QUEUE` (each served by its own handler).
255266
std::set<uint32> m_lastSentFileIds;
267+
std::set<uint32> m_lastSentSharedFileIds;
268+
std::set<uint32> m_lastSentPartFileIds;
256269
};
257270

258271

@@ -263,6 +276,8 @@ m_conn_state(CONN_INIT),
263276
m_passwd_salt(GetRandomUint64()),
264277
m_notification_dispatch_depth(0),
265278
m_lastEcGenSeen(0),
279+
m_lastEcGenSeenShared(0),
280+
m_lastEcGenSeenPart(0),
266281
m_partialUpdateActive(false)
267282
{
268283
wxASSERT(theApp->ECServerHandler);
@@ -731,7 +746,9 @@ static CECPacket *Get_EC_Response_StatRequest(const CECPacket *request, CLoggerA
731746
return response;
732747
}
733748

734-
static CECPacket *Get_EC_Response_GetSharedFiles(const CECPacket *request, CFileEncoderMap &encoders)
749+
static CECPacket *Get_EC_Response_GetSharedFiles(const CECPacket *request, CFileEncoderMap &encoders,
750+
uint64 &io_lastEcGenSeen,
751+
bool partial_update_active, std::set<uint32> &io_lastSentFileIds)
735752
{
736753
wxASSERT(request->GetOpCode() == EC_OP_GET_SHARED_FILES);
737754

@@ -744,29 +761,69 @@ static CECPacket *Get_EC_Response_GetSharedFiles(const CECPacket *request, CFile
744761

745762
encoders.UpdateEncoders();
746763

764+
// Skip-unchanged + EC_TAG_FILE_REMOVED is wired only for the
765+
// `EC_DETAIL_UPDATE` polling path that amuleweb uses (`EC_OP_GET_
766+
// SHARED_FILES` re-issued each cycle with an encoder-retained diff
767+
// state) and only when the client opted into the partial-update
768+
// protocol at auth. `EC_DETAIL_FULL` callers (amulecmd `show shared`,
769+
// any one-shot query) still get every alive file as a full tag.
770+
const bool skip_unchanged_path =
771+
partial_update_active && detail_level == EC_DETAIL_UPDATE;
772+
const uint64 ec_snapshot = skip_unchanged_path
773+
? CKnownFile::GetGlobalECGen() : 0;
774+
const uint64 ec_threshold = io_lastEcGenSeen;
775+
747776
// Snapshot the shared-file list once. GetFileByIndex() does an O(N)
748777
// std::advance over the underlying std::map and re-acquires list_mut
749778
// on every call -- looping it N times is O(N^2) and pegs the main
750779
// thread for tens of minutes on users with tens of thousands of
751780
// shared files (issue #666).
752781
std::vector<CKnownFile*> snapshot;
753782
theApp->sharedfiles->CopyFileList(snapshot);
783+
784+
// Snapshot the alive set for the partial-update removal diff below.
785+
std::set<uint32> current_file_ids;
786+
754787
for (std::vector<CKnownFile*>::const_iterator it = snapshot.begin();
755788
it != snapshot.end(); ++it) {
756789
const CKnownFile *cur_file = *it;
757790

758791
if ( !cur_file || (!queryitems.empty() && !queryitems.count(cur_file->ECID())) ) {
759792
continue;
760793
}
794+
const uint32 ecid = cur_file->ECID();
795+
if (skip_unchanged_path) {
796+
current_file_ids.insert(ecid);
797+
if (cur_file->GetECGen() <= ec_threshold) {
798+
// Client already has the latest exported view of
799+
// this file; absence here is "no change", not
800+
// "deleted" — see `EC_TAG_FILE_REMOVED` emission
801+
// below.
802+
continue;
803+
}
804+
}
761805

762806
CEC_SharedFile_Tag filetag(cur_file, detail_level);
763-
CKnownFile_Encoder *enc = encoders[cur_file->ECID()];
807+
CKnownFile_Encoder *enc = encoders[ecid];
764808
if ( detail_level != EC_DETAIL_UPDATE ) {
765809
enc->ResetEncoder();
766810
}
767811
enc->Encode(&filetag);
768812
response->AddTag(filetag);
769813
}
814+
815+
if (skip_unchanged_path) {
816+
// One EC_TAG_FILE_REMOVED per file that was in the previous
817+
// response but is no longer alive on the server.
818+
for (std::set<uint32>::const_iterator it = io_lastSentFileIds.begin();
819+
it != io_lastSentFileIds.end(); ++it) {
820+
if (!current_file_ids.count(*it)) {
821+
response->AddTag(CECTag(EC_TAG_FILE_REMOVED, *it));
822+
}
823+
}
824+
io_lastSentFileIds.swap(current_file_ids);
825+
io_lastEcGenSeen = ec_snapshot;
826+
}
770827
return response;
771828
}
772829

@@ -933,7 +990,9 @@ static CECPacket *Get_EC_Response_GetClientQueue(const CECPacket *request, CObjT
933990
}
934991

935992

936-
static CECPacket *Get_EC_Response_GetDownloadQueue(const CECPacket *request, CFileEncoderMap &encoders)
993+
static CECPacket *Get_EC_Response_GetDownloadQueue(const CECPacket *request, CFileEncoderMap &encoders,
994+
uint64 &io_lastEcGenSeen,
995+
bool partial_update_active, std::set<uint32> &io_lastSentFileIds)
937996
{
938997
CECPacket *response = new CECPacket(EC_OP_DLOAD_QUEUE);
939998

@@ -944,29 +1003,64 @@ static CECPacket *Get_EC_Response_GetDownloadQueue(const CECPacket *request, CFi
9441003

9451004
encoders.UpdateEncoders();
9461005

1006+
// Skip-unchanged + EC_TAG_FILE_REMOVED is wired only for the
1007+
// `EC_DETAIL_UPDATE` polling path that amuleweb uses, and only when
1008+
// the client opted into the partial-update protocol at auth. Other
1009+
// callers still get every alive file as a full tag.
1010+
const bool skip_unchanged_path =
1011+
partial_update_active && detail_level == EC_DETAIL_UPDATE;
1012+
const uint64 ec_snapshot = skip_unchanged_path
1013+
? CKnownFile::GetGlobalECGen() : 0;
1014+
const uint64 ec_threshold = io_lastEcGenSeen;
1015+
9471016
// Snapshot once to avoid re-locking downloadqueue's mutex on every
9481017
// iteration (see Get_EC_Response_GetSharedFiles for the matching
9491018
// shared-files fix in issue #666).
9501019
std::vector<CPartFile*> snapshot;
9511020
theApp->downloadqueue->CopyFileList(snapshot);
1021+
1022+
std::set<uint32> current_file_ids;
1023+
9521024
for (std::vector<CPartFile*>::const_iterator it = snapshot.begin();
9531025
it != snapshot.end(); ++it) {
9541026
CPartFile *cur_file = *it;
9551027

9561028
if ( !queryitems.empty() && !queryitems.count(cur_file->ECID()) ) {
9571029
continue;
9581030
}
1031+
const uint32 ecid = cur_file->ECID();
1032+
if (skip_unchanged_path) {
1033+
current_file_ids.insert(ecid);
1034+
if (cur_file->GetECGen() <= ec_threshold) {
1035+
// Client already has the latest exported view of
1036+
// this partfile; absence here is "no change",
1037+
// not "deleted" — see `EC_TAG_FILE_REMOVED`
1038+
// emission below.
1039+
continue;
1040+
}
1041+
}
9591042

9601043
CEC_PartFile_Tag filetag(cur_file, detail_level);
9611044

962-
CPartFile_Encoder * enc = static_cast<CPartFile_Encoder *>(encoders[cur_file->ECID()]);
1045+
CPartFile_Encoder * enc = static_cast<CPartFile_Encoder *>(encoders[ecid]);
9631046
if ( detail_level != EC_DETAIL_UPDATE ) {
9641047
enc->ResetEncoder();
9651048
}
9661049
enc->Encode(&filetag);
9671050

9681051
response->AddTag(filetag);
9691052
}
1053+
1054+
if (skip_unchanged_path) {
1055+
for (std::set<uint32>::const_iterator it = io_lastSentFileIds.begin();
1056+
it != io_lastSentFileIds.end(); ++it) {
1057+
if (!current_file_ids.count(*it)) {
1058+
response->AddTag(CECTag(EC_TAG_FILE_REMOVED, *it));
1059+
}
1060+
}
1061+
io_lastSentFileIds.swap(current_file_ids);
1062+
io_lastEcGenSeen = ec_snapshot;
1063+
}
9701064
return response;
9711065
}
9721066

@@ -1623,12 +1717,16 @@ CECPacket *CECServerSocket::ProcessRequest2(const CECPacket *request)
16231717
//
16241718
case EC_OP_GET_SHARED_FILES:
16251719
if ( request->GetDetailLevel() != EC_DETAIL_INC_UPDATE ) {
1626-
response = Get_EC_Response_GetSharedFiles(request, m_FileEncoder);
1720+
response = Get_EC_Response_GetSharedFiles(request, m_FileEncoder,
1721+
m_lastEcGenSeenShared,
1722+
m_partialUpdateActive, m_lastSentSharedFileIds);
16271723
}
16281724
break;
16291725
case EC_OP_GET_DLOAD_QUEUE:
16301726
if ( request->GetDetailLevel() != EC_DETAIL_INC_UPDATE ) {
1631-
response = Get_EC_Response_GetDownloadQueue(request, m_FileEncoder);
1727+
response = Get_EC_Response_GetDownloadQueue(request, m_FileEncoder,
1728+
m_lastEcGenSeenPart,
1729+
m_partialUpdateActive, m_lastSentPartFileIds);
16321730
}
16331731
break;
16341732
//

‎src/ExternalConnector.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ class CaMuleExternalConnector : public wxApp
145145
void GetCommand(const wxString &prompt, char* buffer, size_t buffer_size);
146146
const CECPacket *SendRecvMsg_v2(const CECPacket *request) { return m_ECClient->SendRecvPacket(request); }
147147
void SendPacket(const CECPacket *request) { m_ECClient->SendPacket(request); }
148+
bool IsServerPartialUpdateActive() const { return m_ECClient->ServerSupportsPartialUpdate(); }
148149
void ConnectAndRun(const wxString &ProgName, const wxString& ProgVersion);
149150
void ShowGreet();
150151

‎src/webserver/src/WebServer.h‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -274,32 +274,71 @@ class UpdatableItemsContainer : public ItemsContainer<T> {
274274

275275
/*!
276276
* Process answer of update request, create list of new items for
277-
* full request later. Also remove items that no longer exist in core
277+
* full request later. Also remove items that no longer exist in core.
278+
*
279+
* Partial-update protocol negotiated at auth: when the server is
280+
* partial-update-capable, files unchanged since the last cycle
281+
* are simply absent from the response; deletions arrive as
282+
* explicit `EC_TAG_FILE_REMOVED` markers. The bulk "anything
283+
* missing == deleted" loop below would otherwise wipe most of
284+
* the library every cycle on big libraries (#713). When the
285+
* server is not partial-update-capable (or didn't echo the
286+
* capability), the encoder's per-file diff makes unchanged
287+
* tags effectively empty alive-markers and the bulk-deletion
288+
* loop stays correct.
278289
*/
279290
void ProcessUpdate(const CECPacket *reply, CECPacket *full_req, int req_type)
280291
{
292+
const bool partial_update = this->m_webApp->IsServerPartialUpdateActive();
293+
281294
std::set<I> core_files;
295+
std::set<I> removed_files;
282296
for (CECPacket::const_iterator it = reply->begin(); it != reply->end(); ++it) {
283-
G *tag = (G *) & *it;
297+
const CECTag *raw_tag = &*it;
298+
if (raw_tag->GetTagName() == EC_TAG_FILE_REMOVED) {
299+
// Explicit deletion marker from a partial-
300+
// update-capable server.
301+
removed_files.insert(raw_tag->GetInt());
302+
continue;
303+
}
304+
G *tag = (G *) raw_tag;
284305

285-
core_files.insert(tag->ID());
306+
if (!partial_update) {
307+
core_files.insert(tag->ID());
308+
}
286309
if ( m_items_hash.count(tag->ID()) ) {
287310
T *item = m_items_hash[tag->ID()];
288311
item->ProcessUpdate(tag);
289312
} else {
290313
full_req->AddTag(CECTag(req_type, tag->ID()));
291314
}
292315
}
316+
293317
std::list<I> del_ids;
294-
for(typename std::list<T>::iterator j = this->m_items.begin(); j != this->m_items.end(); ++j) {
295-
if ( core_files.count(j->ID()) == 0 ) {
296-
// item may contain data that need to be freed externally, before
297-
// dtor is called and memory freed
318+
if (partial_update) {
319+
// Only delete what the server explicitly told us to
320+
// delete. Skipped early when there's nothing to
321+
// remove this cycle (the common case).
322+
if (!removed_files.empty()) {
323+
for(typename std::list<T>::iterator j = this->m_items.begin(); j != this->m_items.end(); ++j) {
324+
if (removed_files.count(j->ID())) {
325+
T *real_ptr = &*j;
326+
this->ItemDeleted(real_ptr);
327+
del_ids.push_back(j->ID());
328+
}
329+
}
330+
}
331+
} else {
332+
for(typename std::list<T>::iterator j = this->m_items.begin(); j != this->m_items.end(); ++j) {
333+
if ( core_files.count(j->ID()) == 0 ) {
334+
// item may contain data that need to be freed externally, before
335+
// dtor is called and memory freed
298336

299-
T *real_ptr = &*j;
300-
this->ItemDeleted(real_ptr);
337+
T *real_ptr = &*j;
338+
this->ItemDeleted(real_ptr);
301339

302-
del_ids.push_back(j->ID());
340+
del_ids.push_back(j->ID());
341+
}
303342
}
304343
}
305344
for(typename std::list<I>::iterator j = del_ids.begin(); j != del_ids.end(); ++j) {

0 commit comments

Comments
 (0)