search

rhtconsulting / puppet-jboss_admin

Puppet module for runtime configuration of a JBoss container
MIT License
15 stars 12 forks source link

Prefetch all resources from server in single operation #72

Closed cpitman closed 8 years ago

cpitman commented 9 years ago

This is a performance improvement for the current CLI provider. Instead of doing a read-resource for each resource, it does a single recursive read-resource from the root, and then searches for each resource within the result. I've tested this locally on some simple manifests, and it appears to be working. However, it is a large change to how things work. @itewk can you test if this works for you?

I'm using the quickstart_logging test case to profile this. What I am timing is the length of a run after all the resources exist. My expectation is that the exact same amount of time will be shaved off from an initial run. Before the change, it takes 11.5 seconds. After the change it takes 2.7 seconds.

This appears to save roughly .5 seconds per resource.

itewk commented 9 years ago

@cpitman

I'll pull this in and test against my uses cases.

cpitman commented 9 years ago

Yep, good catch. I've added the same logic to jboss_batch. Again, it appears to work on a simple test case.

Honestly, I'm mostly worried about whether or not the CLI is going to throw some sort of syntax we've never seen before when doing a full container configuration parsing. I already had to make a change here for '(' and ')' characters that I'm a little nervous about. The actual batching of reading is pretty straightforward.

itewk commented 9 years ago

@cpitman I'll put it into my larger test environment that I am building out and see what happens.

As a heads up though I likely wont be able to finish testing this week and I am on vacation next week, so it maybe a bit before I can get us some good results.

~Ian

itewk commented 9 years ago

@cpitman any idea what could be causing the issue I am seeing with this branch?