Skip to content

Commit 2b6a1e2

Browse files
committed
Refactoring
1. Remove recursion of operator++(), this leads to constant memory usage rather than filling the stack at some point 2. Extract subroutines from operator++()
1 parent 9349396 commit 2b6a1e2

2 files changed

Lines changed: 122 additions & 62 deletions

File tree

include/openPMD/ReadIterations.hpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
*/
2121
#pragma once
2222

23+
#include "openPMD/auxiliary/Option.hpp"
2324
#include "openPMD/Iteration.hpp"
2425
#include "openPMD/Series.hpp"
2526

@@ -90,6 +91,12 @@ class SeriesIterator
9091
m_currentIteration = *m_iterationsInCurrentStep.begin();
9192
return true;
9293
}
94+
95+
auxiliary::Option< SeriesIterator * > nextIterationInStep( Iteration & );
96+
97+
auxiliary::Option< SeriesIterator * > nextStep( Iteration & );
98+
99+
auxiliary::Option< SeriesIterator * > loopBody();
93100
};
94101

95102
/**

src/ReadIterations.cpp

Lines changed: 115 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -140,70 +140,59 @@ SeriesIterator::SeriesIterator( Series series )
140140
}
141141
}
142142

143-
SeriesIterator & SeriesIterator::operator++()
143+
auxiliary::Option< SeriesIterator * >
144+
SeriesIterator::nextIterationInStep( Iteration & currentIteration )
144145
{
145-
if( !m_series.has_value() )
146+
using ret_t = auxiliary::Option< SeriesIterator * >;
147+
148+
m_iterationsInCurrentStep.pop_front();
149+
if( m_iterationsInCurrentStep.empty() )
146150
{
147-
*this = end();
148-
return *this;
151+
return ret_t{};
149152
}
150-
Series & series = m_series.get();
151-
auto & iterations = series.iterations;
152-
auto & currentIteration = iterations[ m_currentIteration ];
153+
153154
auto oldIterationIndex = m_currentIteration;
155+
m_currentIteration = *m_iterationsInCurrentStep.begin();
156+
auto & series = m_series.get();
154157

155-
m_iterationsInCurrentStep.pop_front();
156-
if( !m_iterationsInCurrentStep.empty() )
158+
switch( series.iterationEncoding() )
157159
{
158-
m_currentIteration = *m_iterationsInCurrentStep.begin();
159-
switch( series.iterationEncoding() )
160-
{
161-
case IterationEncoding::groupBased:
162-
case IterationEncoding::variableBased:
160+
case IterationEncoding::groupBased:
161+
case IterationEncoding::variableBased:
162+
{
163+
if( !currentIteration.closed() )
163164
{
164-
if( !currentIteration.closed() )
165-
{
166-
currentIteration.get().m_closed =
167-
internal::CloseStatus::ClosedInFrontend;
168-
}
169-
auto begin = series.iterations.find( oldIterationIndex );
170-
auto end = begin;
171-
++end;
172-
series.flush_impl(
173-
begin,
174-
end,
175-
FlushLevel::UserFlush,
176-
/* flushIOHandler = */ true );
177-
178-
series.iterations[ m_currentIteration ].open();
179-
return *this;
165+
currentIteration.get().m_closed =
166+
internal::CloseStatus::ClosedInFrontend;
180167
}
181-
case IterationEncoding::fileBased:
182-
if( !currentIteration.closed() )
183-
{
184-
currentIteration.close();
185-
}
186-
series.iterations[ m_currentIteration ].open();
187-
series.iterations[ m_currentIteration ].beginStep();
188-
return *this;
189-
}
190-
throw std::runtime_error( "Unreachable!" );
191-
}
168+
auto begin = series.iterations.find( oldIterationIndex );
169+
auto end = begin;
170+
++end;
171+
series.flush_impl(
172+
begin,
173+
end,
174+
FlushLevel::UserFlush,
175+
/* flushIOHandler = */ true );
192176

193-
// The currently active iterations have been exhausted.
194-
// Now see if there are further iterations to be found.
195-
196-
if( series.iterationEncoding() == IterationEncoding::fileBased )
197-
{
198-
// this one is handled above, stream is over once it proceeds to here
199-
*this = end();
200-
return *this;
177+
series.iterations[ m_currentIteration ].open();
178+
return { this };
201179
}
202-
203-
if( !currentIteration.closed() )
204-
{
205-
currentIteration.close();
180+
case IterationEncoding::fileBased:
181+
if( !currentIteration.closed() )
182+
{
183+
currentIteration.close();
184+
}
185+
series.iterations[ m_currentIteration ].open();
186+
series.iterations[ m_currentIteration ].beginStep();
187+
return { this };
206188
}
189+
throw std::runtime_error( "Unreachable!" );
190+
}
191+
192+
auxiliary::Option< SeriesIterator * > SeriesIterator::nextStep(
193+
Iteration & currentIteration )
194+
{
195+
using ret_t = auxiliary::Option< SeriesIterator * >;
207196

208197
// since we are in group-based iteration layout, it does not
209198
// matter which iteration we begin a step upon
@@ -222,18 +211,19 @@ SeriesIterator & SeriesIterator::operator++()
222211
* Fallback implementation: Assume that each step corresponds
223212
* with an iteration in ascending order.
224213
*/
225-
auto it = iterations.find( m_currentIteration );
226-
auto itEnd = iterations.end();
214+
auto & series = m_series.get();
215+
auto it = series.iterations.find( m_currentIteration );
216+
auto itEnd = series.iterations.end();
227217
if( it == itEnd )
228218
{
229219
*this = end();
230-
return *this;
220+
return { this };
231221
}
232222
++it;
233223
if( it == itEnd )
234224
{
235225
*this = end();
236-
return *this;
226+
return { this };
237227
}
238228
m_iterationsInCurrentStep = { it->first };
239229
}
@@ -242,8 +232,52 @@ SeriesIterator & SeriesIterator::operator++()
242232
if( status == AdvanceStatus::OVER )
243233
{
244234
*this = end();
245-
return *this;
235+
return { this };
246236
}
237+
238+
return ret_t{};
239+
}
240+
241+
auxiliary::Option< SeriesIterator * > SeriesIterator::loopBody()
242+
{
243+
using ret_t = auxiliary::Option< SeriesIterator * >;
244+
245+
Series & series = m_series.get();
246+
auto & iterations = series.iterations;
247+
auto & currentIteration = iterations[ m_currentIteration ];
248+
249+
250+
{
251+
auto option = nextIterationInStep( currentIteration );
252+
if( option.has_value() )
253+
{
254+
return option;
255+
}
256+
}
257+
258+
// The currently active iterations have been exhausted.
259+
// Now see if there are further iterations to be found.
260+
261+
if( series.iterationEncoding() == IterationEncoding::fileBased )
262+
{
263+
// this one is handled above, stream is over once it proceeds to here
264+
*this = end();
265+
return { this };
266+
}
267+
268+
if( !currentIteration.closed() )
269+
{
270+
currentIteration.close();
271+
}
272+
273+
{
274+
auto option = nextStep( currentIteration );
275+
if( option.has_value() )
276+
{
277+
return option;
278+
}
279+
}
280+
247281
currentIteration.setStepStatus( StepStatus::DuringStep );
248282

249283
auto iteration = iterations.at( m_currentIteration );
@@ -253,12 +287,31 @@ SeriesIterator & SeriesIterator::operator++()
253287
}
254288
else
255289
{
256-
// we had this one already, skip it
257-
// @todo remove recursive call
290+
// we had this iteration already, skip it
258291
iteration.endStep();
259-
return operator++();
292+
return ret_t{}; // empty, go into next iteration
260293
}
261-
return *this;
294+
return { this };
295+
}
296+
297+
SeriesIterator & SeriesIterator::operator++()
298+
{
299+
if( !m_series.has_value() )
300+
{
301+
*this = end();
302+
return *this;
303+
}
304+
auxiliary::Option< SeriesIterator * > res;
305+
/*
306+
* loopBody() might return an empty option to indicate a skipped iteration.
307+
* Loop until it returns something real for us.
308+
*/
309+
do
310+
{
311+
res = loopBody();
312+
} while( !res.has_value() );
313+
314+
return *res.get();
262315
}
263316

264317
IndexedIteration SeriesIterator::operator*()

0 commit comments

Comments
 (0)