Skip to content

/ servo

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

Refactor how LayoutContext structure works (reduce TLS lookups + simplify fns used by seq/parallel code paths). #3069

Merged
merged 1 commit into from Aug 11, 2014
Merged

Refactor how LayoutContext structure works (reduce TLS lookups + simplify fns used by seq/parallel code paths). #3069

merged 1 commit into from Aug 11, 2014

Conversation

@glennw
Copy link
Member

glennw commented Aug 11, 2014

  • LayoutContext is renamed to SharedLayoutContext.
  • SharedLayoutContext is immutable.
  • LayoutContext is a wrapper around SharedLayoutContext + access to local caches (font, style etc).
  • Creating a LayoutContext does a single local_data lookup to fetch the cache information.
  • Android shares same implementation of context.rs as other platforms.
  • LayoutContext can be used from both green thread (parallel layout) and native thread (sequential layout).
  • Removes the need for other types (such as FontContext, StyleSharingCandidateCache etc) to be passed around.
@gw3583
Refactor how LayoutContext structure works (reduce TLS lookups + simp…
Loading status checks…
4a0e01b 
…lify fns used by seq/parallel code paths).

- LayoutContext is renamed to SharedLayoutContext.
- SharedLayoutContext is immutable.
- LayoutContext is a wrapper around SharedLayoutContext + access to local caches (font, style etc).
- Creating a LayoutContext does a single local_data lookup to fetch the cache information.
- Android shares same implementation of context.rs as other platforms.
- LayoutContext can be used from both green thread (parallel layout) and native thread (sequential layout).
- Removes the need for other types (such as FontContext, StyleSharingCandidateCache etc) to be passed around.
@highfive
Copy link

highfive commented Aug 11, 2014

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Aug 11, 2014

Critic review: https://critic.hoppipolla.co.uk/r/2326

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.
@glennw
Copy link
Member Author

glennw commented Aug 11, 2014

r? @pcwalton @metajack @larsbergstrom
pcwalton added a commit that referenced this pull request Aug 11, 2014
@pcwalton
Merge pull request #3069 from glennw/refactor_layout_context
Loading status checks…
8e59706 
Refactor how LayoutContext structure works (reduce TLS lookups + simplify fns used by seq/parallel code paths).
@pcwalton pcwalton merged commit 8e59706 into servo:master Aug 11, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@glennw glennw deleted the glennw:refactor_layout_context branch Sep 15, 2014
@stshine stshine mentioned this pull request Feb 7, 2017
Merged
3 of 5 tasks complete
bors-servo added a commit that referenced this pull request Feb 7, 2017
@bors-servo
Auto merge of #15417 - stshine:column-flexbox, r=<try>
Loading status checks…
8892cb2 
layout: Remove cached thread local context from LayoutContext, and use LayoutContext for assign_inline_sizes()

<!-- Please describe your changes on the following line: -->

According to #3069 the cached thread local context is introduced for green threads. Now green threads has gone, and the existence of cache force us to create a `LayoutContext`, an `AssignISizes` and an `AssignBSizes` for each flow, so the pull request tries to remove it. And it also switch `assign_inline_sizes()` to accept a `LayoutContext` parameter, as according to my current design we need to do full layout to some flex items for column flexbox.

Part of #14123.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because refactoring

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15417)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request Feb 8, 2017
@bors-servo
Auto merge of #15417 - stshine:column-flexbox, r=emilio
Loading status checks…
e2b494b 
layout: Remove cached thread local context from LayoutContext, and use LayoutContext for assign_inline_sizes()

<!-- Please describe your changes on the following line: -->

According to #3069 the cached thread local context is introduced for green threads. Now green threads has gone, and the existence of cache force us to create a `LayoutContext`, an `AssignISizes` and an `AssignBSizes` for each flow during parallel layout, so the pull request tries to remove it. And it also switch `assign_inline_sizes()` to accept a `LayoutContext` parameter, as according to my current design we need to do full layout to some flex items for column flexbox during assign isize traversal.

Part of #14123.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because refactoring

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15417)
<!-- Reviewable:end -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 8, 2017
@stshine
servo: Merge #15417 - layout: Remove cached thread local context from…
f3c730e 
… LayoutContext, and use LayoutContext for assign_inline_sizes() (from stshine:column-flexbox); r=emilio

<!-- Please describe your changes on the following line: -->

According to servo/servo#3069 the cached thread local context is introduced for green threads. Now green threads has gone, and the existence of cache force us to create a `LayoutContext`, an `AssignISizes` and an `AssignBSizes` for each flow during parallel layout, so the pull request tries to remove it. And it also switch `assign_inline_sizes()` to accept a `LayoutContext` parameter, as according to my current design we need to do full layout to some flex items for column flexbox during assign isize traversal.

Part of #14123.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because refactoring

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: e2b494b1d08b8929ca7e5ae369304f41af81ace2

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 9cd92ed44acfaa3b2ba04e345b33d21aff3d752c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@glennw @highfive @hoppipolla-critic-bot @pcwalton @gw3583
You can’t perform that action at this time.