Closed Bug 785268 Opened 13 years ago Closed 13 years ago

IonMonkey: Improve style/design of profiling

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: u443197, Assigned: u443197)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
No description provided.
Attached patch patch (v2)Splinter Review
Attachment #654848 - Attachment is obsolete: true
Attachment #654853 - Flags: review?(dvander)
Comment on attachment 654853 [details] [diff] [review] patch (v2) Review of attachment 654853 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for doing this! ::: js/src/ion/IonInstrumentation.h @@ +20,5 @@ > + > +class IonInstrumentation : public BaseInstrumentation > +{ > + jsbytecode **trackedPc_; > + public: nit: newline above public: ::: js/src/ion/IonMacroAssembler.h @@ +523,5 @@ > + return ret; > + } > + > + // see above comment for what is returned > + uint32 callWithExitFrame(IonCode *target, Register dynStack) { Yay, this seems a lot better. @@ +535,5 @@ > private: > + // These two functions are helpers used around call sites throughout the > + // assembler. They are called from the above call wrappers to emit the > + // necessary instrumentation. > + void leaveBeforeCall() { These names are a little vague, it's not clear what "leave" or "call" refers to. Maybe something like enter/ExitSpsFrame or enter/leaveProfilingFrame? @@ +539,5 @@ > + void leaveBeforeCall() { > + if (!sps_ || !sps_->enabled()) > + return; > + GeneralRegisterSet regs(Registers::TempMask); > + Register r = regs.getAny(); I would just use ReturnReg or CallTempReg0 here, or: Register r = GeneralRegisterSet(Registers::TempMask).getAny(); @@ +552,5 @@ > + GeneralRegisterSet regs(Registers::TempMask & ~Registers::JSCallMask & > + ~Registers::CallMask); > + if (regs.empty()) { > + regs = GeneralRegisterSet(Registers::TempMask); > + Register r = regs.getAny(); Same here.
Attachment #654853 - Flags: review?(dvander) → review+
Status: UNCONFIRMED → RESOLVED
Closed: 13 years ago
Resolution: --- → WORKSFORME
Resolution: WORKSFORME → FIXED
Assignee: general → alex
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: