Artemis icon indicating copy to clipboard operation
Artemis copied to clipboard

Dont emit signals for bytecode in ignored files

Open budde377 opened this issue 11 years ago • 3 comments

Right now we are emitting a signal, from QWebExecutionListener, and then checking whether we should ignore that file. Wouldn't it be better if we kept the list of ignored URL's in QWebExecutionListener, given the high price on emitting signals? Or are there some architectural complications?

budde377 avatar Apr 01 '13 10:04 budde377

The only downside I can think of is a slight increase in the API size between the two parts and a small increase in the responsibilities of the execution listener. I am all for decreasing the number of signals emitted. However we should do some benchmarking first, to ensure that we are not optimizing something prematurely.

If the signals turns out to be as expensive as we think they are, we could move the ExecutionResult object creation to WebKit, and then pull the result out of WebKit after Artemis have executed a sequence of events. This will remove nearly all of the signals being emitted. Again, we should benchmark this stuff before doing anything, it could be that network overhead and page rendering shadows the signal cost.

sema avatar Apr 01 '13 17:04 sema

I have done a bit of profiling using the webpage provided in issue #38. Right now there seems to be two major causes to the slowdown encountered on large pages: (1) the DOM traversal and (2) bytecode coverage.

The DOM traversal seems to be caused by a traversal covering a high number of unnecessary nodes. We should be able to make this much quicker by limiting the traversal.

The bytecode coverage part is not concentrated around one single operation. The poor performance seems to be caused by signalling, qhash of QUrl, ignoring of URLs ect. So no single root cause.

sema avatar Apr 02 '13 15:04 sema

I have committed a couple of performance related changes. However, bytecode coverage (and line coverage) is still causing some slowdown.

sema avatar Apr 03 '13 17:04 sema