Try to improve page loading fluency

[pavelz]

See the first post in this ticket for complete log of Slack/IRC channel discussion leading to this effort.

Background

It is a fact that Adblock Browser lags and falters when loading a lengthy webpage. It is a prominent user objection in Appstore reviews. As Kitt Core developers, we believe that it's caused by the extension practice of loading all the thousands of hiding CSS selectors at once (means, in one event loop run) into every frame.

UPDATE
CSS injection was a false suspect. Generalized the summary accordingly. See https://issues.adblockplus.org/ticket/4249#comment:5 for further background.

What to change

Make the CSS selector loading asynchronous. Actual method and implementation TBD.

[pavelz]

pavel-zdenek [11:33 AM]
So we have two options for future development: redo the whole UI which doesn’t seem to bother the user pool very much, or sit down and think about what could be changed in the extension (and the native code subsequently) to improve the overall swiftness

[11:34]
If we spend several months redoing the UI now, we’ll be again facing the app speed as the greatest roadblock

[11:35]
The current UI is not great, but it’s not terrible either

[11:35]
It’s definitely not the prime reason for sub-5 reviews

mario [11:35 AM]

"and think about what could be changed in the extension"

[11:36]
pavel-zdenek: Can you tell which functions/features actually slow down ABB?

[11:36]
pavel-zdenek: Or would you be able to find out?

[11:38]
pavel-zdenek: FYI, some of the users specifically point out slowness during general usage like scrolling/reading articles. This doesn't necessarily sound like it's caused by the extension.

pavel-zdenek [11:41 AM]
Unfortunately it is. We know pretty good since a long time ago. It’s the method of pulling the thousands CSS rules into every loaded frame, at once. If a page is infinite scroller - i.e. new frames are loaded on the fly, like our beloved mashable.com, that’s exactly the faltering seen by users.

mario [11:42 AM]
pavel-zdenek: gotcha

pavel-zdenek [11:42 AM]
There are two ways to improve:

[11:43]

• make the rule list smaller. 20% less rules would make more than 20% perceivable improvement IMHO

[11:45]

• draw the rules in chunks and apply them in chunks, asynchronously. It may result in jumping layout on some pages, when exactly some postponed chunk applies to the particular page, but it’s still better if the user has partially loaded and responsive page than having no page and waiting.

mario [11:47 AM]
pavel-zdenek: So right at the moment ABB freezes (or rather is blocked from further loading the page) as long as the CSS rules are injected?

pavel-zdenek [11:49 AM]
WebKit is a single thread engine - on iOS at least. So yes, when it is applying thousands of CSS rules at once, it cannot continue layouting and rendering the page, even if the asset loading continues, because protocol handler has separate worker threads.

[11:52]
I’ll tell you a secret. We even considered forking ABP and try to apply some changes in that regard ourselves :slightly_smiling_face: But you are giving us enough work for such stunt to be never practically presentable to you. Now the question is, what should be the next effort with greatest yield of user experience improvement. Extension optimization would be far greater pain on your side than UI polishing, that’s undisputable.

pavel-zdenek [11:55 AM]
did let the cat out of the bag and gets back to his day off now, whistling along the way :slightly_smiling_face:

mario [12:04 PM]
pavel-zdenek: Hehe, thanks for the input. I very much like the idea. Especially since it sounds like you already have a proper idea of where to look and what to try. I'm gonna align this with our plans and get back to you as soon as you return from your days off.

[12:05]
pavel-zdenek: And now enjoy your day off :)

---

pavel-zdenek [11:27 AM]
@mario: what we basically need is to know whether it would get bigger priority than UI overhaul (because if it doesn’t, it’s basically zero priority due to the UI overhaul length) and if our activity on the source code would be endorsed, or you are willing to do the changes yourself - with much greater speed and correctness, indeed

mario [11:28 AM]
pavel-zdenek: Do you have an idea of how much time you'd need to invest? Are we speaking of 2 weeks, 4 weeks, 8 weeks?

[11:30]
pavel-zdenek: Nothing speaks agains contributing to the actual ABP project. However if we intend to introduce changes which are only beneficial for ABB we'd probably need to find another solution. The adblockplusadblockbrowserios project might probably be our best bet in this case. However this depends on the changes.

pavel-zdenek [12:27 PM]
@mario: let’s say a month

[12:27]
Re benefit scope

[12:29]
The overall performance issue of single threaded engine applies to any browser so far, multi threaded compositing is experimental

[12:29]
So throwing several smaller CSS blocks instead on one huge ​_could_​ be beneficial across platforms

[12:30]
But it’s a theory, where the practical effect may be negligible on desktop browsers anyway

[12:30]
So let’s be realistic and handle the case as if it was beneficial for ABB only

[12:33]
Then the question is if such kind of logical change can be abstracted enough so that it’s containable in adblockplusadblockbrowserios

mario [1:14 PM]
pavel-zdenek: Let's do this. Talked to the rest of the team and we're all fine to look into this before we tackle the UI. This gives us additional time to prepare the UI changes.

[pavelz]

​https://www.pivotaltracker.com/story/show/127172209

[pavelz]

Copy pasting from 2016-08-31 meeting notes
There is no particular website which was performing badly that users complained about or that we know of. So we need to go with “the obvious” (Mario):
​http://cnet.net - lots of content, lots of ads
​http://about.com - lots of content, lots of ads
Any NSFW or movie streaming website

UPDATE for some technical reasons beyond my understanding, no CSS collapsing is applied on those pages, so mashable.com stays the primary test case

[pavelz]

CSS injection was a red herring. A default English filterlist, which results to ~15000 selectors, is injected in under 50ms. Specifically on iPhone 6 it's about 30ms. Anyway further profiling turned out a different bottleneck: to satisfy the blocking logic of chrome.webRequest.onBeforeRequest, every outgoing request must be delayed a little bit to call from native code through background JS and back. While a single JS call to onBeforeRequest listener takes at most low single ms (typically 1-3), a relevant dispatch from native code takes typically 30x as much. For a real world test of mashable.com, for about 30-40 requests, the total delay goes from 120ms to almost 3 seconds.

This is not a BLOCKING delay - it's a total of little delays on each little request. The page keeps loading all the time. But the delay naturally accumulates toward the page finish, so in case of unfortunate resource queueing, the effect may be very visible.

The supposed solution aligns with the currently ongoing WKWebView migration. The grand reason for the 30x delay inflation is that both content and background webview share the same Webkit worker thread (webthread), which simply has no time left for our extra background script calls, as it is fully occupied with rendering the webpage. WKWebView will give a whole separate execution process to the background script, so there should be no such delay.

[pavelz]

WKWebView effort merged, evaluated and measured.
TL;DR: not the right way, not faster than UIWebView, with disappointing load scaling pattern

I did two rounds of test of UIWebView and WKWebView builds, 4 different websites, each 3 loads, grand total of 48 page loads. ​https://docs.google.com/spreadsheets/d/1KT2vijJN212gE3EW0Pcx1N_Lq0mOKN40PhdvT_tXTFU
WKWebView slows down roughly by twice as much time per blocking onBeforeRequest roundtrip, compared to UIWebView.

In UIWebView, the slow down is caused by the need to switch from app's main thread to WebView thread. In WKWebView, there is no thread switching in the context of ABB app process, but WKWebView is actually a different process, so the time is taken up by IPC. And it's generally twice as much time (per each request).

What's worse, WKWebView is incapable of handling a significant number of concurrent evaluateJavascript calls. Where UIWebView handles up to 50 concurrent calls without any visible slow down, WKWebView keeps pace only up to about 10 calls and then it goes south. Normally one call takes about 100ms on iPhone6, but at 64 concurrent calls, one takes up to 4.5s (yes seconds). The penalty starts dropping only as the number of concurrent calls is dropping eventually.

So when a website is actually well thought out and carefully coded, so that it does not block itself internally and is capable of firing everything at once, such website totally freezes in ABB and starts appearing eventually after couple of seconds. This can be seen in the testing matrix above for site about.com. The average delay of one request is double or more of other sites - seen in both test rounds.

The above bottleneck could be resolved by applying a limiter to the number of concurrent evaluateJavascript calls. But even at its best, WKWebView will be roughly as fast as UIWebView. IPC is simply killing it. Furthermore, being a standalone process, WKWebView requires an observation logic, so that we know it crashed and be able to restart it and reattach to it.

Conclusion: not worth the effort :(

[pavelz]

Next proposal: JavaScriptCore

It is possible to instantiate a standalone JavaScript context in iOS, through direct access to WebKit's underlying interpreter.

Advantages:

• no thread switching penalty. Instantiated and being accessed from main thread.
• uses the same objects as we are getting already from UIWebView interface - easy integration

Disadvantages
Being a "naked" interpreter. It has none of the Browser Object Model goodies that Chrome extension is used to. The bare minimum would have to be reimplemented, injected and supported from the native side. Here we need an ABP expert feedback: what the background script expects and needs from the JS context? We can think of:

1. XMLHttpRequest already available and used, so that we can actually control it from native code and allow XSS
2. console.* dtto, so that it appears in native logs
3. setTimeout/Interval not available but seems to have existing implementation(s) ​https://github.com/artemyarulin/JSCoreBom
4. window/document not available and scary, due to being a God Object with millions of potentially used features
5. ???

[pavelz]

JavaScriptContext was verified to be running in main thread. Point 3. setTimeout was implemented along the way. setInterval can be either a polyfill or a similar native-backed implementation - no big deal. So the point 4 window/document remains to be answered. Tagging blocked by need of technical feedback.

We could indeed dive in the code ourselves and make educated assumptions, but a core dev feedback would be A) faster B) safer.

[pavelz]

I have skimmed the background script files for some obvious keywords and listing the thoughts.

Roughly in order of possibility, easy first, impossible last.

• setTimeout needs to implement the optional argument passing beyond the timeout number - seen in multiple places

• setInterval will be done as polyfill, needs just the plain form (no args)

• console needs log, error, trace, in both concat and comma separated param forms

• window object must exist at least with the following properties:
  • setTimeout - no issue
  • navigator.userAgent - reasonably easy
  • addEventListener("unload") will be polyfilled but dummy as we are not unloading background script, unless the whole extension is being taken down and the listener action doesn't matter anyway
  • addable ext property - no issue

• window object is also tested for Notification member in notificationHelper.showNotification code can this be ignored? (notifications are not implemented at all)

• existing polyfills:
  1. Promise understood, does not exist in JSC, ABP capable of running just with the polyfill?
  2. fetch understood, needed as well indeed
  3. URL not understood. Is this really an URL polyfill? Minimal necessary? Does not have many of URL functions. Unfortunately uses, for some not entirely clear purpose, document.createHTMLDocument which we cannot provide

• document is used in 5 different scopes. 2 POSSIBLE, 3 IMPOSSIBLE
  1. utils.runAsync postponing callback to after document.readyState == 'loading'. This function also hooks (add/removeEventListener) on "DOMContentLoaded" which, apparently, we will have to simulate. readyState, listener and DOMContentLoaded event is doable.
  2. fetch polyfill asking for document.URL, comparing protocol to chrome-extension. Must be simulated (JSC does not have any base URL per se)
  3. filterValidation.isValidCSSSelector where style element is created and querying attempted. I don't understand the call paths enough to say whether it is needed/called in ABB. Why would ABB need to validate CSS selectors, if the only selectors are from ready made authorized filter lists? Anyway, if it would be needed, we cannot provide such method of selector validation
  4. icon.renderFrames to create canvas element. This should not be needed in ABB. Again cannot provide.
  5. URL polyfill to create completely new standalone document Absolutely impossible

• Image used in icon.loadImage Cannot provide, chances are that it's needed only for browser/page action (hence not needed for ABB)

• URL has polyfill, but that polyfill uses unachievable document functionality

[pavelz]

Technical feedback needed for: A) ensuring completeness of the list B) knowing whether impossible functions are avoidable in ABB scope. Because if not, JSC is unusable and this effort can be abandoned.

[fhd]

I'd have to check for what APIs we use all the same (don't know them by heart), but I can say what is or isn't critical:

1. window.Notification is not critical, but we have fallback code for notifications that attempts to use window.confirm as a last resort. As you said, notifications aren't properly implemented at this stage, so they shouldn't show up at all. The best way to go ahead here is probably to provide a stub for window.Notification that will do nothing for now. Note however that window.Notification is a fallback already, what we really want to use here is chrome.notifications. But we check OS and Chrome version before we attempt to use it, see: ​https://github.com/adblockplus/adblockpluschrome/blob/master/lib/notificationHelper.js#L37

2. URL polyfill: The createHTMLDocument approach is actually a bit of a hack, creating an anchor element just to use it for basic URL parsing. I think it'll be easiest for you to provide a URL object that has the required properties, i.e. href, protocol, hostname, host, pathname and search. (We could at some point run into the situation where we update the extension and it started to use an additional property, which would have it go down the polyfill code path. Shouldn't be that much of an issue to debug and fix in ABB, but it feels just slightly wrong to leave this kind of bug waiting to happen around...)

3. The fetch polyfill is using document.URL to get an absolute URL for internal resources when we pass in a relative one. Only example I could find: The core code tries to fetch subscriptions.xml, which is part of the extension. Shouldn't be too hard to address.

4. CSS validation (i.e. filterValidation.isValidCSSSelector) - this is only needed for the Chrome UI, where users can enter custom filters. One day we want to support this functionality in ABB for iOS too, but even then, I'm not sure we would run this particular code path. Let's not mind for now.

5. Icon stuff (icon.renderFrame and icon.loadImage) this is code that renders the icon in Chrome/Opera/Safari, not at all needed in ABB, this code path will not run.

I think that should answer everything you've found.

BUT: We should possibly take a minute and consider a different approach. What you now want to do is to run the background script of Chrome extensions using JavaScriptCore. That should work, but it might require a bunch of hacks to replicate all the background page functionality extensions (specifically ABP) need.

An alternative approach would be to leave the Chrome extension support path, and use the ABP core code as a library instead. We have a C++ wrapper library for it, which is called libadblockplus, see: ​https://github.com/adblockplus/libadblockplus

We have begun to work on an iOS wrapper library in the same vein, about a year ago. Hasn't been used in production yet and probably needs more work, but if this library approach seems better to you, together we should be able to do this.

[pavelz]

An alternative approach would be to leave the Chrome extension support path, and use the ABP core code as a library instead. We have a C++ wrapper library for it, which is called libadblockplus, see: ​​https://github.com/adblockplus/libadblockplus

@fhd: Yes that's what i accidentaly found out that it exists and it immediately looked like a solution to me. The API has everything we need. We'd just need to replace the wrapper code with Swift. However, @mario has consulted some guy called Sergey, and the conclusion was

It is not as well maintained as ABP for Chrome and might bear performance risks. It’s not advised to be used for now.

Recorded in ABB/ABP meeting minutes of November 23
​https://docs.google.com/document/d/1aWYE8GEgJBs84eQi26Cxub1piS5HCbrjxdvAy6zh5Wo/edit

Mind sorting this out? Using an existing DOM-less build of the extension would be a perfect shortcut.
I am putting the DOM-compatible effort on hold for now and leaving blocked tag in place.

[fhd]

Well it is surely not usable as is, pretty much just a prototype. On the other hand, you are about to tackle pretty much the same effort (except there wouldn't be the Chrome API in the middle), so I was thinking it could be an option to work on this together.

IIRC, our approach was not to write a Obj-C wrapper for libadblockplus itself, but rather have a mixed C++/Obj-C code base in the wrapper, and use JavaScriptCore instead of V8 under the hood. That way, we wouldn't have to start from scratch, but could reuse most of the logic we already have in libadblockplus. Whether that's actually the right way to do this, I'm not sure, we'd have to see (and we would probably want to use Swift in this day and age, indeed).

Anyway, logically this library approach makes sense IMO, it's closer to how we normally do things:

adblockpluscore is where all the core ad blocking, filter and subscription management code is. It's not a real library, more of a shared component. But by now it's pretty well decoupled from the rest.

adblockpluschrome uses adblockpluscore and the Chrome extension API to build an actual ad blocker. So it runs the core code in the extension environment, works around platform specific issues, and so on.

libadblockplus is essentially a C++ wrapper around adblockpluscore, but it is more library-like.

adblockplusie uses libadblockplus and the IE extension API to build an ad blocker, much like adblockpluschrome does with adblockpluscore.

There's more, but that's it, in a nut shell.

So based on that, it seems logical to have a library like libadblockplus-ios or libadblockplus-swift or whatever, and use it directly to support the ad blocking functionality in the browser. That makes particular sense because we are working on rewriting the memory and performance sentitive parts of the core code in native code, which native clients could particularly benefit from.

But I see two problems:

1. We don't know our way around iOS so well. Sergei has been working on this because he is the module owner of libadblockplus, and he would be needed for interaction with our code. But he alone could probably not do this in reasonable time. You on the other hand know your way around iOS and are going to do almost that now, anyway. So maybe it makes sense for us to join forces on that.

2. There is actually a bit of application logic in adblockpluschrome, adblockplusie and ultimately all platform specific code bases.
   • Some of that is unnecessary duplication we need to get rid of but don't get around to. Since you are interacting with the core code directly, we've been handling this in api.js. If we go for the libadblockplus-ios approach, we would handle it there. (Then, one day, we would move that logic into adblockpluscore where it really belongs, but you would ideally not even notice.)
   • Some of that is not avoidable, and by having you support the Chrome extension API, so far the theory, you wouldn't have to worry about this part. But in practice, I get the impression, you do have to worry about it a lot. And then you either have to do weird workarounds (since there is the Chrome API in the middle), or we have to make changes to our Chrome extension, which is not great either.

So, the problems are probably not that bad, but I guess it's still not such an easy decision. To me, it ultimately depends on what you think is going to work out best. If you'd like to investigate the library approach more, I'll reach out to Sergei so we can discuss it.

[pavelz]

@fhd what works the best is a solution which A. delays web requests as little as possible B. its memory consumption is as much under control as possible. UIWebView turned out to be quite bad in A. and the worst in B. WKWebView is way better in B, but even worse than UIWebView on A. We are hoping for JSC being better in both. A REALLY REALLY BEST would be a blocking engine written in native code, of course.

So i need to know which part of libabp is exactly "prototypish". JS? The native wrapper? Sergey's advice sounded like it's the JS part - supported by claiming "performance risks" - which rhymes with JS to me. You sound more like that JS is pretty standard stuff, built out of standard components, shared with other ABP targets and platforms. Where "performance" is still risky, but it's not a problem of libabp per se, rather the whole platform, and you are tackling that independently of the particular libabp build.

Because, this is the decision point for us: does existence of libabp mean that you can produce a JS core which functionally covers the existing api.js, WITHOUT relying on any of the BOM/DOM magic smoke? Then filling in whatever C++/Swift/ObjC glue looks like way safer itinerary, even without knowing what exactly and in which language needs to be done. It does not need to be decided now. We can even start with taking libadblockplus as is and compile it in with just thin calling convention wrapper. Xcode compiles and links C++ just fine.

There is many more questions still: what does the core use for filterlist storage? What does it use for async event notifications, like popup detection result? What do we do about notification related feature requests like #3900 #3905?

But if you KNOW these are not feature blockers or even relevant to libadblockplus, don't need to answer here. This will require a new thread anyway when it starts happening.

[fhd]

libadblockplus itself is fairly stable, what's prototype-ish is the iOS/Obj-C wrapper in the ios branch: ​https://github.com/adblockplus/libadblockplus/tree/ios

And yes, this will work as a native library, no magic required - none outside the library, anyway.

libadblockplus is written in such a way that the client provides hooks for storing filters, sending requests, showing notifications etc. I suggest you have a look at libadblockplus as-is, the API is reasonably well documented for our standards :) Drop me a line (ideally on IRC) if you have any questions.

The one problem I see with libadblockplus is that it uses bundles v8 and uses it internally. We have to rip that part out and use JavaScriptCore instead. That's the part Sergei has begun to work on in the ios branch. Best ask him directly if you have questions about that.

[pelereinewi]

spam

[mikeshean11]

spam

[peter11]

spam

[kagisoallen60]

spam

[faizi0021]

spam

[booms1]

spam

[Saracovar]

spam

[ghdsports]

spam