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.

Sign up for GitHub

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

Jump to bottom

Introduce cache to speed things up and cleanup method #171

Merged
merged 1 commit into from
Jul 19, 2021
Merged

Introduce cache to speed things up and cleanup method #171

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 17 additions & 2 deletions lib/facterdb.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ def self.database
@database ||= "[#{facterdb_fact_files.map { |f| read_json_file(f) }.join(',')}]\n"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the way this should have been probably in a synchronized block, but that's another story.

end

# @note Call this method at the end of test suite, for example via after(:suite), to reclaim back the memory required to hold json data and filter cache
def self.cleanup
@database = nil
Thread.current[:facterdb_last_filter_seen] = nil
Thread.current[:facterdb_last_facts_seen] = nil
end

# @return [Boolean] - returns true if we should use the default facterdb database, false otherwise
# @note If the user passes anything to the FACTERDB_SKIP_DEFAULTDB environment variable we assume
# they want to skip the default db
Expand Down Expand Up @@ -123,8 +130,16 @@ def self.valid_filters?(filters)

# @return [Array] - array of hashes of facts
# @param filter [Object] - The filter to convert to jgrep string
def self.get_facts(filter=nil)
def self.get_facts(filter=nil, cache=true)
if cache && filter && filter == Thread.current[:facterdb_last_filter_seen]
return Thread.current[:facterdb_last_facts_seen]
end
filter_str = generate_filter_str(filter)
JGrep.jgrep(database, filter_str).map { |hash| Hash[hash.map{ |k, v| [k.to_sym, v] }] }
result = JGrep.jgrep(database, filter_str).map { |hash| Hash[hash.map{ |k, v| [k.to_sym, v] }] }
Copy link
Member

Choose a reason for hiding this comment

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

Note to self: this is why only the keys are symbols while it should really be something like deep_symbolize or use something like https://api.rubyonrails.org/classes/ActiveSupport/HashWithIndifferentAccess.html

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 do not understand the implementation enough, but my general rule of thumb is - for big dictionaries, stay away from symbolizing/indifferent access it can be quite slow. I suggest to use just strings and convert all input to strings as well if possible.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but that's an API change that's quite big. That's why HashWithIndifferentAccess is probably a cleaner solution.

if cache
Thread.current[:facterdb_last_filter_seen] = filter
Thread.current[:facterdb_last_facts_seen] = result
end
result
end
end