Better deal with changes to the ResourceLoadStatisticsStore file on disk. We should merge the data on disk with the one we have in memory instead of merely replacing our in-memory data. This avoids data loss when re-syncing from disk.
Created attachment 315396 [details] Patch
Created attachment 315398 [details] Patch
Comment on attachment 315398 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315398&action=review Looks good! R=me > Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:130 > + int m_monthDay { 0 }; // [1, 31]. Is this really 1-based? If we are, why is t it initialized with a 1?
Comment on attachment 315398 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315398&action=review >> Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:130 >> + int m_monthDay { 0 }; // [1, 31]. > > Is this really 1-based? If we are, why is t it initialized with a 1? Yes, dayInMonthFromDayInYear() returns a value that is 1-based. The default constructor does not construct a valid date so I do not think it matters that we initialize all members to 0 though. The default constructor is only used for the KeyedDecoding and we would not use the date if the decoding failed.
Comment on attachment 315398 [details] Patch Clearing flags on attachment: 315398 Committed r219499: <http://trac.webkit.org/changeset/219499>
All reviewed patches have been landed. Closing bug.
Comment on attachment 315398 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315398&action=review > Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:141 > + std::merge(existingDates.begin(), existingDates.end(), newDates.begin(), newDates.end(), mergedDates.begin()); I’d be tempted to assert std::is_sorted before calling std::merge. > Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:147 > + // Remove duplicate dates. > + removeRepeatedElements(mergedDates); > + > + // Drop old dates until the Vector size reaches operatingDatesWindow. > + while (mergedDates.size() > operatingDatesWindow) > + mergedDates.remove(0); (When researching this, I discovered that Vector::removeRepeatedElements calls resize instead of calling shrink. It should call shrink.) This code is unnecessarily inefficient. We should resize only once, not multiple times, and we should reduce the amount we move elements in the vector. Here is the almost identical code I would write that resizes the vector at most once and moves items in the vector at most twice: // Remove duplicate dates, then throw away all but the newest dates that fit in the window size. auto begin = mergedDates.begin(); auto end = std::unique(begin, mergedDates.end()); if (end - begin > operatingDatesWindow) end = std::move(end - operatingDatesWindow, end, begin); mergedDates.shrink(end - begin);