Ticket #10354 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Optimise SaveParameterFile

Reported by: Harry Jeffery Owned by: Harry Jeffery
Priority: major Milestone: Release 3.3
Component: Framework Keywords:
Cc: Blocked By:
Blocking: Tester: Anders Markvardsen

Description (last modified by Harry Jeffery) (diff)

SaveParameterFile's performance now significantly lags behind LoadParameterFile's performance. The poor performance appears to be coming from Poco's XML::Node::appendChild method which adds a child in O(n) time, taking up approximately (80%) of the execution time.

Change History

comment:1 Changed 6 years ago by Harry Jeffery

Refs #10354 Fix bug in ProgressBase

ProgressBase would fail to report progress after a call to resetNumSteps. This was due to m_last_reported being left at a high value, filtering any new progress reports.

We now properly reset the state during resetNumSteps.

Changeset: 145256b3d3feef0a34f120446f03a012b74ce7d5

comment:2 Changed 6 years ago by Harry Jeffery

Refs #10354 Optimise SaveParameterFile

Poco was hopeless. The only way to get around the O(n) bottleneck required accessing private members of Poco::XML::Node.

As the XML output was relatively simple, we can instead just write the same thing ourselves, manually.

On my machine this brings SaveParameterFile's performance to be the same as LoadParameterFile's performance, rather than 5-10x slower.

Changeset: 4274857cd98e0e9a1a9f5c0e21f229cf73b5e58b

comment:3 Changed 6 years ago by Harry Jeffery

  • Status changed from new to assigned

comment:4 Changed 6 years ago by Harry Jeffery

  • Status changed from assigned to inprogress

comment:5 Changed 6 years ago by Harry Jeffery

Refs #10354 Improve SaveParameterFile accuracy

  • Output detector id when possible
  • Do not output insignificant position differences
  • Do not try to be too clever with relative parent positions

Changeset: 1c4365378e99f9ab52eeae2b6dea076d267f38a3

comment:6 Changed 6 years ago by Harry Jeffery

  • Description modified (diff)

comment:7 Changed 6 years ago by Harry Jeffery

  • Description modified (diff)

comment:8 Changed 6 years ago by Harry Jeffery

Merge branch 'master' into feature/10354_optimise_saveparameterfile

Refs #10354

Conflicts:

Code/Mantid/Framework/DataHandling/src/SaveParameterFile.cpp

Changeset: 126bbdd5d2753c96546709a722f06ae7ec6c1678

comment:9 Changed 6 years ago by Harry Jeffery

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

Testing

This should not affect the behaviour or output of SaveParameterFile in any way, except to make it quicker.

A good workspace to demonstrate the improvement on is MAPS00018314.nxs from systemtests.

It's worth nothing that there is a known bug (existing before this ticket's changes): Saving, then loading, then saving location again causes the locations to move. This is being investigated in a separate ticket (#10524)

comment:10 Changed 6 years ago by Anders Markvardsen

  • Status changed from verify to verifying
  • Tester set to Anders Markvardsen

comment:11 Changed 6 years ago by Harry Jeffery

  • Status changed from verifying to closed

Merge branch 'master' into feature/10354_optimise_saveparameterfile

Refs #10354

Conflicts:

Code/Mantid/Framework/DataHandling/src/SaveParameterFile.cpp

Full changeset: ​126bbdd5d2753c96546709a722f06ae7ec6c1678

comment:12 Changed 6 years ago by Anders Markvardsen

Merge remote-tracking branch 'origin/feature/10354_optimise_saveparameterfile'

Full changeset: ​3b17eb9f446a3845498e80681db36b0b76425e7a

comment:13 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​11196

Note: See TracTickets for help on using tickets.