Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory optimization for online feature extraction of long recordings #3038

Merged
merged 7 commits into from
Mar 6, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[feat] RecyclingVector for optional memory saving
  • Loading branch information
Piotr Żelasko committed Feb 15, 2019
commit 04c263688b4145345bf034e2a5fa5d26916c0a7d
8 changes: 7 additions & 1 deletion src/feat/feature-window.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ struct FrameExtractionOptions {
BaseFloat blackman_coeff;
bool snip_edges;
bool allow_downsample;
int max_feature_vectors;
// May be "hamming", "rectangular", "povey", "hanning", "blackman"
// "povey" is a window I made to be similar to Hamming but to go to zero at the
// edges, it's pow((0.5 - 0.5*cos(n/N*2*pi)), 0.85)
Expand All @@ -59,7 +60,8 @@ struct FrameExtractionOptions {
round_to_power_of_two(true),
blackman_coeff(0.42),
snip_edges(true),
allow_downsample(false) { }
allow_downsample(false),
max_feature_vectors(-1) { }

void Register(OptionsItf *opts) {
opts->Register("sample-frequency", &samp_freq,
Expand Down Expand Up @@ -90,6 +92,10 @@ struct FrameExtractionOptions {
opts->Register("allow-downsample", &allow_downsample,
"If true, allow the input waveform to have a higher frequency than "
"the specified --sample-frequency (and we'll downsample).");
opts->Register("max-feature-vectors", &max_feature_vectors,
"Memory optimization. If larger than 0, periodically remove feature "
"vectors so that only this number of the latest feature vectors is "
"retained.");
}
int32 WindowShift() const {
return static_cast<int32>(samp_freq * 0.001 * frame_shift_ms);
Expand Down
43 changes: 37 additions & 6 deletions src/feat/online-feature.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,50 @@

namespace kaldi {

RecyclingVector::RecyclingVector(int items_to_hold) : items_to_hold_(items_to_hold),
first_available_index_(0) { }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a KALDI_ASSERT(items_to_hold > 0);. The behavior would be fascinating if this is zero. : )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 - eagle eyes ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've actually mapped 0 to -1, hopefully making it more user-friendly than program termination.


RecyclingVector::~RecyclingVector() {
DeletePointers(&items_);
}

Vector<BaseFloat> *RecyclingVector::Retrieve(int index) const {
if (index < first_available_index_) {
KALDI_ERR << "Attempted to retrieve feature vector that was "
"already removed by the RecyclingVector";
}
// 'at' does size checking.
return items_.at(index - first_available_index_);
}

void RecyclingVector::Store(Vector<BaseFloat> *item) {
// Internally, we keep at most 2x requested items_to_hold_ and clean up
// when this number is reached
const int size_before_erase = items_.size();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming the concern here is the efficiency, as to avoid droping the first element and moving the entire rig towards the head of the vector on every (overflowing) insertion. If so, consider std::deque as the internal container instead of the std::vector. Presumably, a library-implemented container maintains a good balance between overhead and efficiency of at() access. 2x the requested size may not always be a good heuristic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered this before but wasn't sure. Since you're the second person to suggest it and it results in less code, I guess it's better though.

if (size_before_erase == 2 * items_to_hold_) {
items_.erase(items_.begin(), items_.begin() + items_to_hold_);
first_available_index_ += (size_before_erase - items_.size());
}
items_.push_back(item);
}

int RecyclingVector::Size() const {
return first_available_index_ + items_.size();
}


template<class C>
void OnlineGenericBaseFeature<C>::GetFrame(int32 frame,
VectorBase<BaseFloat> *feat) {
// 'at' does size checking.
feat->CopyFromVec(*(features_.at(frame)));
feat->CopyFromVec(*(features_.Retrieve(frame)));
};

template<class C>
OnlineGenericBaseFeature<C>::OnlineGenericBaseFeature(
const typename C::Options &opts):
computer_(opts), window_function_(computer_.GetFrameOptions()),
input_finished_(false), waveform_offset_(0) { }
input_finished_(false), waveform_offset_(0),
features_(opts.frame_opts.max_feature_vectors) { }

template<class C>
void OnlineGenericBaseFeature<C>::AcceptWaveform(BaseFloat sampling_rate,
Expand Down Expand Up @@ -63,11 +95,10 @@ template<class C>
void OnlineGenericBaseFeature<C>::ComputeFeatures() {
const FrameExtractionOptions &frame_opts = computer_.GetFrameOptions();
int64 num_samples_total = waveform_offset_ + waveform_remainder_.Dim();
int32 num_frames_old = features_.size(),
int32 num_frames_old = features_.Size(),
num_frames_new = NumFrames(num_samples_total, frame_opts,
input_finished_);
KALDI_ASSERT(num_frames_new >= num_frames_old);
features_.resize(num_frames_new, NULL);

Vector<BaseFloat> window;
bool need_raw_log_energy = computer_.NeedRawLogEnergy();
Expand All @@ -81,7 +112,7 @@ void OnlineGenericBaseFeature<C>::ComputeFeatures() {
// note: this online feature-extraction code does not support VTLN.
BaseFloat vtln_warp = 1.0;
computer_.Compute(raw_log_energy, vtln_warp, &window, this_feature);
features_[frame] = this_feature;
features_.Store(this_feature);
}
// OK, we will now discard any portion of the signal that will not be
// necessary to compute frames in the future.
Expand Down
33 changes: 27 additions & 6 deletions src/feat/online-feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,31 @@ namespace kaldi {
/// @{


/// This class serves as a storage for feature vectors with an option to limit
/// the memory usage by removing old elements. The deleted frames indices are
/// "remembered" so that regardless of the MAX_ITEMS setting, the user always
/// provides the indices as if no deletion was being performed.
/// This is useful when processing very long recordings which would otherwise
/// cause the memory to eventually blow up when the features are not being removed.
class RecyclingVector {
public:
RecyclingVector(int items_to_hold = -1);

Vector<BaseFloat> *Retrieve(int index) const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this return a const Vector<BaseFloat> &? The object owns the vectors outright. This way they could not be deleted by mistake.

Also, I would maybe call the methods At and PushBack, so they hark back to std::vector.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for At and PushBack.

If we want to return const & here I'd go full way and store the Vector<BaseFloat>s by value - but last time I checked move operations were still not supported in Kaldi, and I didn't want to make this PR bigger than necessary.

I don't want to go half-way because what prevents somebody from PushBacking a null pointer, or deleting the object after he pushes it in here? Would gladly use unique_ptr, but didn't see it used in the codebase.

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree, it should better be symmetric.


void Store(Vector<BaseFloat> *item);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document it very clearly that the item becomes owned by this collection and must not be deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1


int Size() const;

~RecyclingVector();

private:
std::vector<Vector<BaseFloat>*> items_;
int items_to_hold_;
int first_available_index_;
};


/// This is a templated class for online feature extraction;
/// it's templated on a class like MfccComputer or PlpComputer
/// that does the basic feature extraction.
Expand All @@ -61,7 +86,7 @@ class OnlineGenericBaseFeature: public OnlineBaseFeature {
return computer_.GetFrameOptions().frame_shift_ms / 1000.0f;
}

virtual int32 NumFramesReady() const { return features_.size(); }
virtual int32 NumFramesReady() const { return features_.Size(); }

virtual void GetFrame(int32 frame, VectorBase<BaseFloat> *feat);

Expand All @@ -88,10 +113,6 @@ class OnlineGenericBaseFeature: public OnlineBaseFeature {
ComputeFeatures();
}

~OnlineGenericBaseFeature() {
DeletePointers(&features_);
}

private:
// This function computes any additional feature frames that it is possible to
// compute from 'waveform_remainder_', which at this point may contain more
Expand All @@ -107,7 +128,7 @@ class OnlineGenericBaseFeature: public OnlineBaseFeature {

// features_ is the Mfcc or Plp or Fbank features that we have already computed.

std::vector<Vector<BaseFloat>*> features_;
RecyclingVector features_;

// True if the user has called "InputFinished()"
bool input_finished_;
Expand Down