From 3dbf5e727d2875feebafdf542acd71d7bad5f83f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 3 Jun 2019 00:18:52 +0200 Subject: [PATCH] process in incoming order (reverse-chronological order if backward) makes code simpler, don't need fix to undo reverse ordering of nonOverlappingEvents. reverse looking is very likely premature optimization as well. --- .../room/timeline/persistence/GapWriter.js | 16 +++------------- .../storage/idb/stores/TimelineEventStore.js | 10 ++++------ 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/src/matrix/room/timeline/persistence/GapWriter.js b/src/matrix/room/timeline/persistence/GapWriter.js index 9d22407a..bcd3f6ea 100644 --- a/src/matrix/room/timeline/persistence/GapWriter.js +++ b/src/matrix/room/timeline/persistence/GapWriter.js @@ -10,28 +10,18 @@ export default class GapWriter { } // events is in reverse-chronological order (last event comes at index 0) if backwards async _findOverlappingEvents(fragmentEntry, events, txn) { - const {direction} = fragmentEntry; - // make it in chronological order, so findFirstOrLastOccurringEventId can - // use it's supposed optimization if event ids sorting order correlates with timeline chronology. - // premature optimization? - if (direction.isBackward) { - events = events.slice().reverse(); - } const eventIds = events.map(e => e.event_id); - const findLast = direction.isBackward; let nonOverlappingEvents = events; let neighbourFragmentEntry; - const neighbourEventId = await txn.timelineEvents.findFirstOrLastOccurringEventId(this._roomId, eventIds, findLast); + const neighbourEventId = await txn.timelineEvents.findFirstOccurringEventId(this._roomId, eventIds); if (neighbourEventId) { console.log("_findOverlappingEvents neighbourEventId", neighbourEventId); // trim overlapping events const neighbourEventIndex = events.findIndex(e => e.event_id === neighbourEventId); - const start = direction.isBackward ? neighbourEventIndex + 1 : 0; - const end = direction.isBackward ? events.length : neighbourEventIndex; - nonOverlappingEvents = events.slice(start, end); + nonOverlappingEvents = events.slice(0, neighbourEventIndex); // get neighbour fragment to link it up later on const neighbourEvent = await txn.timelineEvents.getByEventId(this._roomId, neighbourEventId); - console.log("neighbourEvent", {neighbourEvent, start, end, nonOverlappingEvents, events, neighbourEventIndex}); + console.log("neighbourEvent", {neighbourEvent, nonOverlappingEvents, events, neighbourEventIndex}); const neighbourFragment = await txn.timelineFragments.get(this._roomId, neighbourEvent.fragmentId); neighbourFragmentEntry = fragmentEntry.createNeighbourEntry(neighbourFragment); } diff --git a/src/matrix/storage/idb/stores/TimelineEventStore.js b/src/matrix/storage/idb/stores/TimelineEventStore.js index ac4ada9a..7c333245 100644 --- a/src/matrix/storage/idb/stores/TimelineEventStore.js +++ b/src/matrix/storage/idb/stores/TimelineEventStore.js @@ -153,7 +153,7 @@ export default class TimelineEventStore { return events; } - /** Finds the first (or last if `findLast=true`) eventId that occurs in the store, if any. + /** Finds the first eventId that occurs in the store, if any. * For optimal performance, `eventIds` should be in chronological order. * * The order in which results are returned might be different than `eventIds`. @@ -167,7 +167,7 @@ export default class TimelineEventStore { // In that case we could avoid running over all eventIds, as the reported order by findExistingKeys // would match the order of eventIds. That's why findLast is also passed as backwards to keysExist. // also passing them in chronological order makes sense as that's how we'll receive them almost always. - async findFirstOrLastOccurringEventId(roomId, eventIds, findLast = false) { + async findFirstOccurringEventId(roomId, eventIds) { const byEventId = this._timelineStore.index("byEventId"); const keys = eventIds.map(eventId => [roomId, eventId]); const results = new Array(keys.length); @@ -175,9 +175,7 @@ export default class TimelineEventStore { // find first result that is found and has no undefined results before it function firstFoundAndPrecedingResolved() { - let inc = findLast ? -1 : 1; - let start = findLast ? results.length - 1 : 0; - for(let i = start; i >= 0 && i < results.length; i += inc) { + for(let i = 0; i < results.length; ++i) { if (results[i] === undefined) { return; } else if(results[i] === true) { @@ -186,7 +184,7 @@ export default class TimelineEventStore { } } - await byEventId.findExistingKeys(keys, findLast, (key, found) => { + await byEventId.findExistingKeys(keys, false, (key, found) => { const index = keys.indexOf(key); results[index] = found; firstFoundKey = firstFoundAndPrecedingResolved();