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

Optimize load #10

Merged
merged 8 commits into from Oct 12, 2016

Conversation

Projects
None yet
3 participants
@jreidinger
Contributor

jreidinger commented Oct 11, 2016

small step for human, but big step for yast

This optimization is done based on performance testing of big hosts file.

result of performance_test script that I also added in this commit:

previous:

time ruby performance_test/augeas_loader_test.rb 

real    0m14.590s
user    0m14.584s
sys 0m0.004s

after keys cache:

time ruby performance_test/augeas_loader_test.rb 

real    0m7.484s
user    0m7.472s
sys 0m0.008s
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 11, 2016

Coverage Status

Coverage increased (+1.8%) to 76.149% when pulling fe8c709 on optimize_load into a7067c9 on master.

coveralls commented Oct 11, 2016

Coverage Status

Coverage increased (+1.8%) to 76.149% when pulling fe8c709 on optimize_load into a7067c9 on master.

1 similar comment
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 11, 2016

Coverage Status

Coverage increased (+1.8%) to 76.149% when pulling fe8c709 on optimize_load into a7067c9 on master.

coveralls commented Oct 11, 2016

Coverage Status

Coverage increased (+1.8%) to 76.149% when pulling fe8c709 on optimize_load into a7067c9 on master.

@jreidinger jreidinger referenced this pull request Oct 11, 2016

Merged

Optimize hosts read #454

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 12, 2016

Coverage Status

Coverage increased (+2.1%) to 76.487% when pulling 4134947 on optimize_load into a7067c9 on master.

coveralls commented Oct 12, 2016

Coverage Status

Coverage increased (+2.1%) to 76.487% when pulling 4134947 on optimize_load into a7067c9 on master.

1 similar comment
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 12, 2016

Coverage Status

Coverage increased (+2.1%) to 76.487% when pulling 4134947 on optimize_load into a7067c9 on master.

coveralls commented Oct 12, 2016

Coverage Status

Coverage increased (+2.1%) to 76.487% when pulling 4134947 on optimize_load into a7067c9 on master.

jreidinger added some commits Oct 12, 2016

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 12, 2016

Coverage Status

Coverage increased (+2.2%) to 76.554% when pulling b06afe8 on optimize_load into a7067c9 on master.

coveralls commented Oct 12, 2016

Coverage Status

Coverage increased (+2.2%) to 76.554% when pulling b06afe8 on optimize_load into a7067c9 on master.

1 similar comment
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 12, 2016

Coverage Status

Coverage increased (+2.2%) to 76.554% when pulling b06afe8 on optimize_load into a7067c9 on master.

coveralls commented Oct 12, 2016

Coverage Status

Coverage increased (+2.2%) to 76.554% when pulling b06afe8 on optimize_load into a7067c9 on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 12, 2016

Coverage Status

Coverage increased (+2.2%) to 76.554% when pulling b06afe8 on optimize_load into a7067c9 on master.

coveralls commented Oct 12, 2016

Coverage Status

Coverage increased (+2.2%) to 76.554% when pulling b06afe8 on optimize_load into a7067c9 on master.

Show outdated Hide outdated lib/cfa/augeas_parser.rb Outdated
Show outdated Hide outdated lib/cfa/augeas_parser.rb Outdated
def load_value(aug, aug_key, keys_cache)
subkeys = keys_cache.keys_for_prefix(aug_key)
nested = !subkeys.empty?

This comment has been minimized.

@lslezak

lslezak Oct 12, 2016

NP: I'd move the initialization just before where it is used.

@lslezak

lslezak Oct 12, 2016

NP: I'd move the initialization just before where it is used.

This comment has been minimized.

@jreidinger

jreidinger Oct 12, 2016

Contributor

I do not get this note.

@jreidinger

jreidinger Oct 12, 2016

Contributor

I do not get this note.

Show outdated Hide outdated lib/cfa/augeas_parser.rb Outdated
@lslezak

This comment has been minimized.

Show comment
Hide comment
@lslezak

lslezak Oct 12, 2016

BTW do we need a ~400KB testing file in Git? There are just ~25 unique lines + 10000 repeated lines. Could we build the testing file before running the test?

Something like:

header = "..." # probably a HEREDOC...
data_line = "10.254.128.01   pepa1.labs.suse.cz pepa1"
data = ""
10_000.times { data += data_line }

File.write(testing_file, header + data)

lslezak commented Oct 12, 2016

BTW do we need a ~400KB testing file in Git? There are just ~25 unique lines + 10000 repeated lines. Could we build the testing file before running the test?

Something like:

header = "..." # probably a HEREDOC...
data_line = "10.254.128.01   pepa1.labs.suse.cz pepa1"
data = ""
10_000.times { data += data_line }

File.write(testing_file, header + data)
@jreidinger

This comment has been minimized.

Show comment
Hide comment
@jreidinger

jreidinger Oct 12, 2016

Contributor

@lslezak what is benefit? I do not want to affect my performance test by any kind of generation, so it is static file, easy to measure. I think 400kb file is not so horrible.

Contributor

jreidinger commented Oct 12, 2016

@lslezak what is benefit? I do not want to affect my performance test by any kind of generation, so it is static file, easy to measure. I think 400kb file is not so horrible.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 12, 2016

Coverage Status

Coverage increased (+2.2%) to 76.554% when pulling d892547 on optimize_load into a7067c9 on master.

coveralls commented Oct 12, 2016

Coverage Status

Coverage increased (+2.2%) to 76.554% when pulling d892547 on optimize_load into a7067c9 on master.

@lslezak

This comment has been minimized.

Show comment
Hide comment
@lslezak

lslezak Oct 12, 2016

I just found out that the file is not packaged into .gem or .rpm so then it's not critical.

lslezak commented Oct 12, 2016

I just found out that the file is not packaged into .gem or .rpm so then it's not critical.

@lslezak

LGTM

@jreidinger jreidinger merged commit 6142cca into master Oct 12, 2016

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+2.2%) to 76.554%
Details

@jreidinger jreidinger deleted the optimize_load branch Oct 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment