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

Make SHSegmentInfo explicit in CardTable #1505

Open
wants to merge 3 commits into
base: static_h
Choose a base branch
from

Conversation

lavenzg
Copy link
Contributor

@lavenzg lavenzg commented Sep 6, 2024

Summary:
Currently SHSegmentInfo lives in the prefix of CardTable inline
storage (to be specific, prefix of the cards_ array). But this is
only defined in one comment. Add it into a union with the cards_
array to make it clear. It also simplifies the reasoning of following
diffs, in which we need to add more fields to SHSegmentInfo.

In addition, kFirstUsedIndex should take into account of the size of
SHSegmentInfo, since the size of SHSegmentInfo could be larger than
(2 * kCardTableSize) >> kLogCardSize) for small segment size.

Differential Revision: D61747499

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 6, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D61747499

lavenzg added a commit to lavenzg/hermes that referenced this pull request Oct 21, 2024
Summary:

Currently `SHSegmentInfo` lives in the prefix of CardTable inline
storage (to be specific, prefix of the `cards_` array). But this is
only defined in one comment. Add it into a union with the `cards_`
array to make it clear. It also simplifies the reasoning of following
diffs, in which we need to add more fields to `SHSegmentInfo`.

In addition, `kFirstUsedIndex` should take into account of the size of
`SHSegmentInfo`, since the size of `SHSegmentInfo` could be larger than
`(2 * kCardTableSize) >> kLogCardSize)` for small segment size.

Differential Revision: D61747499
lavenzg added a commit to lavenzg/hermes that referenced this pull request Oct 21, 2024
Summary:

Currently `SHSegmentInfo` lives in the prefix of CardTable inline
storage (to be specific, prefix of the `cards_` array). But this is
only defined in one comment. Add it into a union with the `cards_`
array to make it clear. It also simplifies the reasoning of following
diffs, in which we need to add more fields to `SHSegmentInfo`.

In addition, `kFirstUsedIndex` should take into account of the size of
`SHSegmentInfo`, since the size of `SHSegmentInfo` could be larger than
`(2 * kCardTableSize) >> kLogCardSize)` for small segment size.

Differential Revision: D61747499
lavenzg added a commit to lavenzg/hermes that referenced this pull request Oct 21, 2024
Summary:

Currently `SHSegmentInfo` lives in the prefix of CardTable inline
storage (to be specific, prefix of the `cards_` array). But this is
only defined in one comment. Add it into a union with the `cards_`
array to make it clear. It also simplifies the reasoning of following
diffs, in which we need to add more fields to `SHSegmentInfo`.

In addition, `kFirstUsedIndex` should take into account of the size of
`SHSegmentInfo`, since the size of `SHSegmentInfo` could be larger than
`(2 * kCardTableSize) >> kLogCardSize)` for small segment size.

Differential Revision: D61747499
@lavenzg lavenzg force-pushed the export-D61747499-to-static_h branch from 5a67d25 to df295ed Compare October 21, 2024 23:37
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D61747499

lavenzg added a commit to lavenzg/hermes that referenced this pull request Oct 22, 2024
Summary:

Currently `SHSegmentInfo` lives in the prefix of CardTable inline
storage (to be specific, prefix of the `cards_` array). But this is
only defined in one comment. Add it into a union with the `cards_`
array to make it clear. It also simplifies the reasoning of following
diffs, in which we need to add more fields to `SHSegmentInfo`.

In addition, `kFirstUsedIndex` should take into account of the size of
`SHSegmentInfo`, since the size of `SHSegmentInfo` could be larger than
`(2 * kCardTableSize) >> kLogCardSize)` for small segment size.

Differential Revision: D61747499
@lavenzg lavenzg force-pushed the export-D61747499-to-static_h branch from df295ed to b44464a Compare November 8, 2024 01:14
lavenzg added a commit to lavenzg/hermes that referenced this pull request Nov 8, 2024
Summary:

Currently `SHSegmentInfo` lives in the prefix of CardTable inline
storage (to be specific, prefix of the `cards_` array). But this is
only defined in one comment. Add it into a union with the `cards_`
array to make it clear. It also simplifies the reasoning of following
diffs, in which we need to add more fields to `SHSegmentInfo`.

In addition, `kFirstUsedIndex` should take into account of the size of
`SHSegmentInfo`, since the size of `SHSegmentInfo` could be larger than
`(2 * kCardTableSize) >> kLogCardSize)` for small segment size.

Differential Revision: D61747499
lavenzg added a commit to lavenzg/hermes that referenced this pull request Nov 8, 2024
Summary:

Currently `SHSegmentInfo` lives in the prefix of CardTable inline
storage (to be specific, prefix of the `cards_` array). But this is
only defined in one comment. Add it into a union with the `cards_`
array to make it clear. It also simplifies the reasoning of following
diffs, in which we need to add more fields to `SHSegmentInfo`.

In addition, `kFirstUsedIndex` should take into account of the size of
`SHSegmentInfo`, since the size of `SHSegmentInfo` could be larger than
`(2 * kCardTableSize) >> kLogCardSize)` for small segment size.

Differential Revision: D61747499
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D61747499

1 similar comment
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D61747499

lavenzg added a commit to lavenzg/hermes that referenced this pull request Nov 14, 2024
Summary:
Pull Request resolved: facebook#1505

Currently `SHSegmentInfo` lives in the prefix of CardTable inline
storage (to be specific, prefix of the `cards_` array). But this is
only defined in one comment. Add it into a union with the `cards_`
array to make it clear. It also simplifies the reasoning of following
diffs, in which we need to add more fields to `SHSegmentInfo`.

In addition, `kFirstUsedIndex` should take into account of the size of
`SHSegmentInfo`, since the size of `SHSegmentInfo` could be larger than
`(2 * kCardTableSize) >> kLogCardSize)` for small segment size.

Differential Revision: D61747499
Differential Revision: D61675022
…er (facebook#1504)

Summary:
Pull Request resolved: facebook#1504

Large segment needs to be backed by a large storage size.
StorageProvider currently always allocate fixed size of storage
determined by HERMESVM_LOG_HEAP_SEGMENT_SIZE.

This diffs adds support of allocating larger storage with below
changes:
1. `newStorage()` and `deleteStorage()` takes additional `sz` parameter.
2. For `MallocStorageProvider` and `VMAllocateStorageProvider`, simply
change the previous fixed storage size to passed in `sz`.
3. For `ContiguousVAStorageProvider`, use a BitVector to manage
allocations and deallocations. This can be improved later if we observe
fragmentations.

The support of enabling different sizes of heap segment will be added
later.

Differential Revision: D61676721
Summary:
Pull Request resolved: facebook#1505

Currently `SHSegmentInfo` lives in the prefix of CardTable inline
storage (to be specific, prefix of the `cards_` array). But this is
only defined in one comment. Add it into a union with the `cards_`
array to make it clear. It also simplifies the reasoning of following
diffs, in which we need to add more fields to `SHSegmentInfo`.

In addition, `kFirstUsedIndex` should take into account of the size of
`SHSegmentInfo`, since the size of `SHSegmentInfo` could be larger than
`(2 * kCardTableSize) >> kLogCardSize)` for small segment size.

Differential Revision: D61747499
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D61747499

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants