Closed
Bug 513716
Opened 15 years ago
Closed 15 years ago
VMPI_getPrivateResidentPageCount() for Linux assumes pagesize
Categories
(Tamarin Graveyard :: Garbage Collection (mmGC), defect, P3)
Tracking
(Not tracked)
VERIFIED
FIXED
flash10.1
People
(Reporter: stejohns, Assigned: lhansen)
References
Details
Attachments
(3 files)
(deleted),
patch
|
stejohns
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
lhansen
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
edwsmith
:
review+
|
Details | Diff | Splinter Review |
The Linux implementation assumes that the system pagesize matches MMgc pagesize (which is always 4k). This isn't a valid assumption since system pagesize might vary.
Assignee | ||
Comment 1•15 years ago
|
||
Closely related to bug #520080.
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•15 years ago
|
||
Attachment #404214 -
Flags: review?(stejohns)
Assignee | ||
Updated•15 years ago
|
Priority: -- → P3
Target Milestone: --- → flash10.1
Reporter | ||
Comment 3•15 years ago
|
||
Comment on attachment 404214 [details] [diff] [review]
General cleanup around getVMPageSize and getPrivateResidentPageCount
approved with nits:
-- "static long pagesize" -> "static size_t pagesize"
-- since we have VMPI_snprintf we should be using it for all new code and deprecating VMPI_sprintf
Attachment #404214 -
Flags: review?(stejohns) → review+
Assignee | ||
Comment 4•15 years ago
|
||
Good catches both. Will fix before pushing.
Assignee | ||
Comment 5•15 years ago
|
||
redux changeset: 2670:a004bf5fcae0
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 6•15 years ago
|
||
I pushed this fix, to fix the build:
http://hg.mozilla.org/tamarin-redux/rev/a4ca57beee55
as a result, GCHeap::kNativePageSize is now unused in TR. If it's unused everywhere then we should remove it. reopening the bug for post-mortem review of the patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 7•15 years ago
|
||
already pushed, posting here to trigger review.
Attachment #404308 -
Flags: superreview?(lhansen)
Assignee | ||
Updated•15 years ago
|
Attachment #404308 -
Flags: superreview?(lhansen) → superreview+
Assignee | ||
Comment 8•15 years ago
|
||
Clean a little more:
(1) All client code that I have access to reads kNativePageSize through a pointer to a GCHeap instance, so it does not need to be static. That is good, because if it is static then there's a question of order of initialization of kNativePageSize and the cached value in VMPI_getVMPageSize. So make it a non-static propertly.
(2) Nobody uses the abstraction vmPageSize. So remove it.
Attachment #404633 -
Flags: review?(edwsmith)
Updated•15 years ago
|
Attachment #404633 -
Flags: review?(edwsmith) → review+
Assignee | ||
Comment 9•15 years ago
|
||
(In reply to comment #8)
> Created an attachment (id=404633) [details]
> Final cleanup
redux changeset: 2687:7f7ea1a67bf6
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Comment 10•15 years ago
|
||
Resolved fixed engineering / work item that has been pushed. Setting status to verified.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•