Closed Bug 1539482 Opened 7 months ago Closed 5 months ago

Reduce the number of IPC calls on nsISHEntry in nsDocShell::LoadHistoryEntry

Categories

(Core :: Document Navigation, task, P2)

Other Branch
task

Tracking

()

RESOLVED FIXED
Fission Milestone M3

People

(Reporter: annyG, Assigned: annyG)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

No description provided.
Fission Milestone: --- → ?
Summary: Reduce the number of IPC calls on nsISHentry in nsDocShell::LoadHistoryEntry → Reduce the number of IPC calls on nsISHEntry in nsDocShell::LoadHistoryEntry
Fission Milestone: ? → M2
Type: defect → enhancement

In nsDocShell::LoadHistoryEntry method, there are 13 sync IPC calls
on nsISHEntry that retrieve information from the session
history entry and create a doc shell load state object
using the retrieved information. By adding a new method
'CreateLoadInfo'on nsISHEntry, inside of which the
doc shell load state object will be created (with
appropriate data filled out) and returned, we eliminate 12
sync IPC call, resulting in just 1 IPC call to
nsISHEntry::CreateLoadInfo.

Whiteboard: [4/29] patches waiting for review
Whiteboard: [4/29] patches waiting for review → [5/3] patches under review
Fission Milestone: M2 → M3
Attached patch diff.patchSplinter Review

Hi Kyle,

I was wondering if you could help shed any light on the failure I am getting here https://treeherder.mozilla.org/#/jobs?repo=try&revision=c75cc3eb7ef40979defb57553e4b68f65c6f930f&selectedJob=244926456

Just now I have discovered two things in my patch that were wrong, but even with those things fixed, the test fails (I don't have a try run with the new changes added because I haven't had time to get a try run).

The changes are attached in diff.patch

Thanks so much,
Anny

P.S. Clearly that |NS_WARNING("ANNY --- ");| is very necessary.

Flags: needinfo?(kyle)

Do we know if we're ending up with the exact same LoadState object after these improvements?

When I was working on the initial loadstate implementation, I basically had to develop 2 pipelines (keeping the old one around while building a new one) to compare the object I was building with the mess of arguments we were getting from LoadURI to InternalLoad. That exercise might be helpful here, via implementing some sort of object equality test in DocShellLoadState, if that's even possible.

That way, at the end of LoadHistoryEntry, we could check whether our single IPC call DocShellLoadState objects are the same as the old, lots of calls version. I would suspect this will be a handy tool to have later too, since these objects are going to end up all over the place and probably katamari-ing more properties in.

Flags: needinfo?(kyle)
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Whiteboard: [5/3] patches under review
Version: unspecified → Other Branch
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.