-
Notifications
You must be signed in to change notification settings - Fork 743
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
Pre-generate builtins and keywords for lexers #1418
Conversation
Memory profile summary--- master https://travis-ci.org/rouge-ruby/rouge/jobs/642229408
+++ PR https://travis-ci.org/rouge-ruby/rouge/jobs/643419479
- Total allocated: 14.70 MB (142309 objects)
+ Total allocated: 13.53 MB (132177 objects)
- Total retained: 5.31 MB (37531 objects)
+ Total retained: 5.09 MB (35512 objects) |
@ashmaroli Thanks for this—that looks like a nice little reduction. Do you see any issues, @jneen? |
@jneen Requesting your feedback on this so that it can be included in the upcoming release. |
Will this get included in the upcoming release..? |
@ashmaroli Sorry, still waiting for @jneen's feedback. |
Memory profile summary--- master https://travis-ci.com/github/rouge-ruby/rouge/jobs/318551844
+++ PR https://travis-ci.com/github/rouge-ruby/rouge/jobs/318777292
- Total allocated: 14.14 MB (141625 objects)
+ Total allocated: 13.07 MB (132473 objects)
- Total retained: 5.44 MB (37843 objects)
+ Total retained: 5.22 MB (35959 objects) |
@ashmaroli I think this has been in review for long enough. I'm happy to merge this but is there anything else you wanted to add or is it ready from your side, too? |
@ashmaroli It seemed like it would be handy to have a single task that runs all the builtin generators but in the course of testing it, I came across a bunch of bugs in some of the current Rake tasks. I've fixed those and generated new builtin files. Perhaps I should revert the most recent commits and submit as a separate PR but I thought for the moment, I'd just build on your work here. What do you think? |
@pyrmont I think its a good idea to have a One Task to But if the additional changes are going to push this PR to be included in the next month's release rather than today's, I feel it'd be better if the new changes were in a separate PR so that if I were asked to make changes to this proposal at a future date, then it'd be easier for me if I didn't have to wrap my head around your changes as well... |
Yeah, fair enough. Will revert. |
Thanks for the PR, @ashmaroli. I'll open a separate one for the generate task. |
Thank you very much @pyrmont :) |
Rouge has a number of lexers that include a list of the names of built-in functions for more accurate tokenisation. Some of these lists consist of YAML files that need to be computed at runtime. Rather than take this approach, this commit ensures that the lists are plain Ruby files that can be loaded by the respective lexers.
Instead of loading a YAML file and computing the sets from the data at runtime, to reduce memory usage.