Skip to content
New issue

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

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

Already on GitHub? # to your account

Improve replace_with performance #21

Open
gkorland opened this issue Aug 11, 2019 · 6 comments
Open

Improve replace_with performance #21

gkorland opened this issue Aug 11, 2019 · 6 comments
Labels
enhancement New feature or request

Comments

@gkorland
Copy link
Contributor

Current replace_with code seems very inefficient

  1. Find all values.
  2. Compute all paths from values found in (1)
  3. Find values to replace again using paths from (2) to replace
        let paths = {
  (1)          let result = self.select()?;
  (2)         self.compute_paths(result)
        };

        if let Some(ref mut value) = &mut self.value {
            for tokens in paths {
  (3)              replace_value(tokens, value, fun);
            }
        }

It seems like the replace should account in step (1) while running select()

@freestrings
Copy link
Owner

@gkorland yes, I think it's an improvement point too.

the double dot expressions (..) create duplicate reference as results, so deduplication is required when selecting. I didn't think of this when I first implemented the engine and due to the wide range of tests, i am ignoring this issue on 2.x versions.

I'm thinking about use cases like HTTP proxying or batch processing. I'll try to clean up use cases and to plan for performance improvements if necessary. It is going to proceed in 3.x.

this issue should be rethought when implementing 3.x.

@freestrings freestrings added the enhancement New feature or request label Aug 13, 2019
@gkorland
Copy link
Contributor Author

implementing 3.x. did you mean 0.3.x?

@freestrings
Copy link
Owner

yes. 0.3.x

@gkorland
Copy link
Contributor Author

@freestrings when do you plan to release 0.3.0?

@freestrings
Copy link
Owner

@gkorland I am planning a release next month. after finishing testing Nginx module, it will be next month.

@rohitjoshi
Copy link

Is it resolved?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants