Ticket #10370 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

In MergeMDFiles reserve memory for events in one go

Reported by: Ian Bush Owned by: Ian Bush
Priority: major Milestone: Release 3.4
Component: Framework Keywords:
Cc: Alex.Buts@… Blocked By:
Blocking: Tester: Alex Buts

Description

MergeMDFiles, in loadAndAddFrom, reserves memory for events in a box each time it loads a file. This causes the vector to be completely reallocated on every single file load.

This ticket should allow all the memory a vector needs to be reserved in one call.

Change History

comment:1 Changed 6 years ago by Ian Bush

Refs #10370 Added method to MDBox to reserve memory for events.

MergeMDFiles uses this method to reserve memory for events in an MDBox object. Removed reserve in loadAndAddFrom method on MDBox (could this have a detrimental effect on LoadMD?).

Changeset: bede8f882d0810bf8fc06add8164ec414627dc70

comment:2 Changed 6 years ago by Ian Bush

Refs #10370 Added unit test for ReserveMemoryForLoad.

Changeset: c01e4f831e1f58d4638c5a786e438cb536d3eac2

comment:3 Changed 6 years ago by Ian Bush

Refs #10370 Reserve memory again in LoadMD.

Changes to loadAndAddFrom in MDBox meant memory is no longer reserved in LoadMD, this fixes the issue. Note the performance gain for this, if any, appears marginal.

Changeset: c63a2a2144232cf8e1a9354fc2e8e00e3d9abcd3

comment:4 Changed 6 years ago by Ian Bush

  • Status changed from new to assigned

comment:5 Changed 6 years ago by Ian Bush

  • Status changed from assigned to inprogress

comment:6 Changed 6 years ago by Ian Bush

  • Status changed from inprogress to verify
  • Resolution set to fixed

comment:7 Changed 6 years ago by Alex Buts

  • Status changed from verify to verifying
  • Tester set to Alex Buts

comment:8 Changed 6 years ago by Alex Buts

  • Status changed from verifying to reopened
  • Resolution fixed deleted

I think, memory reservation can be done a bit nicer and more reliably without loss of provided performance.

comment:9 Changed 6 years ago by Ian Bush

  • Status changed from reopened to inprogress

Revert "Refs #10370 Reserve memory again in LoadMD."

This reverts commit c63a2a2144232cf8e1a9354fc2e8e00e3d9abcd3. Instead of this we can still allow reserve to be called in loadAndAddFrom method in MDBox.

Changeset: ec902c6e993b6c61a8c550ce22eafb5962ea8da5

comment:10 Changed 6 years ago by Ian Bush

Refs #10370 Added reserve back to loadAndAddfrom in MDBox.

This means the reserve will still be performed if required, but not if memory was already reserved.

Changeset: 54ad8c6ae6f51e6e2710a400c2856ff6a5fcba36

comment:11 Changed 6 years ago by Ian Bush

Revert "Refs #10370 Added reserve back to loadAndAddfrom in MDBox."

This reverts commit 54ad8c6ae6f51e6e2710a400c2856ff6a5fcba36. Decided to leave the reserve as previously, as push back should allow for adequate performance in most cases even if another method relying on loadAndAddFrom in MDBox does not call reserveMemoryForLoad first.

Changeset: 68b05b4ca2a370183219eea23dac91e166870357

comment:12 Changed 6 years ago by Ian Bush

Revert "Revert "Refs #10370 Reserve memory again in LoadMD.""

This reverts commit ec902c6e993b6c61a8c550ce22eafb5962ea8da5.

Changeset: fde2f5abfc38df00c06d87d95df6f04d99fe2ecb

comment:13 Changed 6 years ago by Ian Bush

  • Status changed from inprogress to verify
  • Resolution set to fixed

comment:14 Changed 6 years ago by Alex Buts

  • Status changed from verify to closed

Merge remote-tracking branch 'origin/feature/10370_MergeMDFiles_memory_reserve'

Full changeset: 25e7f04c5468c25ee6f522c06e669a06e8e1d553

comment:15 Changed 6 years ago by Nick Draper

  • Milestone changed from Backlog to Release 3.4

moved to r 3.4 as tickets are closed

comment:16 Changed 5 years ago by Nick Draper

Somehow these slipped through without a resolution. Set to Fixed.

comment:17 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 11212

Note: See TracTickets for help on using tickets.