989344 - IonMonkey: Lower resume points and encode them with RecoverWriter.

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Nicolas B. Pierron [:nbp]]

Currently RecoverWriter is writing one Recover entry per snapshot.  We should share the recover entry among snapshots and encode the recover separately.

Comment 1

[Nicolas B. Pierron [:nbp]]

Attachment: [part 1] Extract LRecover from LSnapshot.

This patch extract the LRecover out of LSnapshot, this patch also separate the function for encoding Recover and Snapshots.

Comment 2

[Nicolas B. Pierron [:nbp]]

Attachment: [part 2] Share LRecover between LSnapshot

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8398940 [details] [diff] [review]
[part 1] Extract LRecover from LSnapshot.

Review of attachment 8398940 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/shared/CodeGenerator-shared.cpp
@@ +311,1 @@
>          recovers_.endFrame();

AFAICS, endFrame doesn't do much. Can we combine startFrame and endFrame as writeFrame()?

@@ +324,5 @@
>  
> +    if (!encode(snapshot->recover()))
> +        return false;
> +
> +    RecoverOffset recoverOffset = snapshot->recover()->recoverOffset();

Nit: MOZ_ASSERT(recoverOffset != INVALID_RECOVER_OFFSET);

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8398958 [details] [diff] [review]
[part 2] Share LRecover between LSnapshot

Review of attachment 8398958 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/shared/Lowering-shared.cpp
@@ +56,5 @@
>      lir->setOperand(inputPosition, LUse(operand->virtualRegister(), LUse::ANY));
>  }
>  
>  LRecover *
>  LIRGeneratorShared::buildRecover(MResumePoint *rp)

Renaming this to getRecover now would make it more clear that it's not always returning a new LRecover.

Comment 5

[Jan de Mooij [:jandem]]

As discussed on IRC, LRecoverInfo would be nice.

Comment 6

[Nicolas B. Pierron [:nbp]]

(In reply to Jan de Mooij [:jandem] from comment #3)
> Comment on attachment 8398940 [details] [diff] [review]
> [part 1] Extract LRecover from LSnapshot.
> 
> ::: js/src/jit/shared/CodeGenerator-shared.cpp
> @@ +311,1 @@
> >          recovers_.endFrame();
> 
> AFAICS, endFrame doesn't do much. Can we combine startFrame and endFrame as
> writeFrame()?

I did so, and removed the nallocs_ field of RecoverWriter, as it is only used within writeFrame.

Comment 7

[Nicolas B. Pierron [:nbp]]

(part 1) https://hg.mozilla.org/integration/mozilla-inbound/rev/2aeb2f123f8e
(part 2) https://hg.mozilla.org/integration/mozilla-inbound/rev/feae125fca6f

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2aeb2f123f8e
https://hg.mozilla.org/mozilla-central/rev/feae125fca6f