Improve js::jit::AssemblerBufferWithConstantPools::hasSpaceForInsts performance
Categories
(Core :: JavaScript Engine: JIT, enhancement, P3)
Tracking
()
People
(Reporter: nbp, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: perf, Whiteboard: [arm64:m4])
The code for s::jit::AssemblerBufferWithConstantPools::hasSpaceForInsts is being executed every time we call vixl::MozBaseAssembler::nextInstrOffset, which implies that we do it for every instruction.
The problem is that this logic is quite complex and adds a lot of code to be executed for each instruction. I suspect that we could simplify this logic by computing ahead the offset at which a pool should be inserted to satisfy all the registered constants.
Therefore replacing all the math of hasSpaceForInsts and moving them to the point where a constant is registered in the constant pool. At the end hasSpaceForInsts should look something along the lines of:
inline bool hasSpaceForInsts(size_t numInsts) {
return nextOffset + numInsts * InstSize < nextPoolOffset;
}
Additionally, we should also move largest/slow functions of AssemblerBufferWithConstantPools to some cpp files.
Comment 1•6 years ago
|
||
For ARM I implemented several optimizations to avoid put-into-buffer overhead, see comments above putInt()
in IonAssemblerBufferWithConstantPools.h. Subsequently, assembly time on ARM was not much of a factor any more, and profiling did not show hasSpaceForInsts()
to take significant time on that platform.
It is possible that the situation is different on ARM64 for other reasons, or that hasSpaceForInsts()
has since grown more complex again and takes more time now, or that I did poor profiling work, but it would be nice to have profiling data to support this bug.
As I wrote in bug 1443082, the vixl assembler is (based on profiling evidence anyway, and for webassembly compilation) a very poor performer, and optimizing that first might be important to get meaningful profiles for hasSpaceForInsts()
.
Reporter | ||
Comment 2•6 years ago
|
||
P3 until we get profiling data to back this guess.
Comment 3•6 years ago
|
||
[arm64:m4] because this bug is not a blocker for ARM64 Fennec riding the trains.
Updated•2 years ago
|
Description
•