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.

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

Reduce allocations in SelectorMap (Rule Hash) code #1274

Closed
pradeep90 opened this issue Nov 18, 2013 · 2 comments
Closed

Reduce allocations in SelectorMap (Rule Hash) code #1274

pradeep90 opened this issue Nov 18, 2013 · 2 comments

Comments

@pradeep90
Copy link
Contributor

@pradeep90 pradeep90 commented Nov 18, 2013

  • Replace insert_or_update_with with find_mut + insert because the default argument to the first function leads to an allocation every time
  • Do the Rule to Declaration conversion in a for loop instead of using map
bors-servo pushed a commit that referenced this issue Nov 22, 2013
Issue #1274

Also:
+ Reduce number of clones of Rule
+ Make Rule have Arc<Selector>
@emberian
Copy link
Contributor

@emberian emberian commented Nov 23, 2013

Probably closedby #1299. I don't understand why you think "Do the Rule to Declaration conversion in a for loop instead of using map" is a good thing, though.

@pradeep90
Copy link
Contributor Author

@pradeep90 pradeep90 commented Nov 24, 2013

@cmr Oh yeah, you're right.

For some strange reason, I thought vec.map() will be more inefficient than for loop / iter().map().
But I see that vec.map() is written in terms of iter().map(f).collect(). Anyway, it is deprecated now.

And like you said, iter().map() will lead to vec using with_capacity instead of possibly allocating one more element each time as in a for loop. I should have stuck to the idiom.

I will change this on Monday. Thanks for the correction.

bors-servo pushed a commit that referenced this issue Nov 25, 2013
bors-servo pushed a commit that referenced this issue Nov 25, 2013
@pradeep90 pradeep90 closed this Nov 26, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.