145791 2015-06-09 01:04:43 -0700 [SOUP] Network Cache: Give more priority to reads over writes in IO WorkQueue 2015-06-09 10:13:09 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED Gtk P2 Normal --- 1 cgarcia webkit-unassigned commit-queue svillar zan oldest_to_newest 1100598 0 cgarcia 2015-06-09 01:04:43 -0700 We could keep the default priority for reads and use idle priority for writes. 1100599 1 254555 cgarcia 2015-06-09 01:09:05 -0700 Created attachment 254555 Patch 1100600 2 commit-queue 2015-06-09 01:11:48 -0700 Attachment 254555 did not pass style-queue: ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:72: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 1100601 3 254555 svillar 2015-06-09 01:15:27 -0700 Comment on attachment 254555 Patch Seems like a good idea, hope it doesn't lead to starvation 1100603 4 cgarcia 2015-06-09 01:18:33 -0700 (In reply to comment #3) > Comment on attachment 254555 [details] > Patch > > Seems like a good idea, hope it doesn't lead to starvation Thanks. Well, this main loop only runs disk cache read/write tasks, so at some point it will finish reading resources from cache and writes will happen, I guess. 1100611 5 254555 zan 2015-06-09 02:17:00 -0700 Comment on attachment 254555 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254555&action=review > Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:268 > runTaskInQueue([channel, offset, data, completionHandler] { > channel->write(offset, data, completionHandler); > - }, queue); > + }, queue, G_PRIORITY_DEFAULT_IDLE); This is never called with a non-null WorkQueue, meaning it's always scheduled on the main context. The point being, is the WorkQueue priority scheduling necessary? 1100612 6 cgarcia 2015-06-09 02:36:27 -0700 (In reply to comment #5) > Comment on attachment 254555 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=254555&action=review > > > Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:268 > > runTaskInQueue([channel, offset, data, completionHandler] { > > channel->write(offset, data, completionHandler); > > - }, queue); > > + }, queue, G_PRIORITY_DEFAULT_IDLE); > > This is never called with a non-null WorkQueue, meaning it's always > scheduled on the main context. The point being, is the WorkQueue priority > scheduling necessary? It's rather the opposite, since it always receives a work queue, it's never scheduled in the main context, but in the work queue main context, and we still want IOChannel::read() to happen before IOChannel::write(). 1100689 7 cgarcia 2015-06-09 10:12:29 -0700 Committed r185366: <http://trac.webkit.org/changeset/185366> 254555 2015-06-09 01:09:05 -0700 2015-06-09 01:15:27 -0700 Patch wk2-disk-cache-io-priority.diff text/plain 4724 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCA5ZWZiYzQ2Li45NzUzN2VjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjMg QEAKKzIwMTUtMDYtMDkgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgorCisgICAgICAgIFtTT1VQXSBOZXR3b3JrIENhY2hlOiBHaXZlIG1vcmUgcHJpb3JpdHkgdG8g cmVhZHMgb3ZlciB3cml0ZXMgaW4gSU8gV29ya1F1ZXVlCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDU3OTEKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBLZWVwIHVzaW5nIEdfUFJJT1JJVFlfREVGQVVMVCBm b3IgcmVhZHMsIGJ1dCB1c2UKKyAgICAgICAgR19QUklPUklUWV9ERUZBVUxUX0lETEUgZm9yIHdy aXRlcyBub3cuCisKKyAgICAgICAgKiBOZXR3b3JrUHJvY2Vzcy9jYWNoZS9OZXR3b3JrQ2FjaGVJ T0NoYW5uZWxTb3VwLmNwcDoKKyAgICAgICAgKFdlYktpdDo6TmV0d29ya0NhY2hlOjpydW5UYXNr SW5RdWV1ZSk6IFVzZSB0aGUgZ2l2ZW4gcHJpb3JpdHkuCisgICAgICAgIChXZWJLaXQ6Ok5ldHdv cmtDYWNoZTo6SU9DaGFubmVsOjpyZWFkKTogUGFzcyBHX1BSSU9SSVRZX0RFRkFVTFQKKyAgICAg ICAgdG8gcnVuVGFza0luUXVldWUoKS4KKyAgICAgICAgKFdlYktpdDo6TmV0d29ya0NhY2hlOjpJ T0NoYW5uZWw6OnJlYWRTeW5jKTogRGl0dG8uCisgICAgICAgIChXZWJLaXQ6Ok5ldHdvcmtDYWNo ZTo6b3V0cHV0U3RyZWFtV3JpdGVSZWFkeUNhbGxiYWNrKTogVXNlCisgICAgICAgIEdfUFJJT1JJ VFlfREVGQVVMVF9JRExFIGZvciB3cml0ZSBhc3luYyBvcGVyYXRpb25zLgorICAgICAgICAoV2Vi S2l0OjpOZXR3b3JrQ2FjaGU6OklPQ2hhbm5lbDo6d3JpdGUpOiBQYXNzCisgICAgICAgIEdfUFJJ T1JJVFlfREVGQVVMVF9JRExFIHRvIHJ1blRhc2tJblF1ZXVlKCkuCisKIDIwMTUtMDYtMDggIENh cmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29tPgogCiAgICAgICAgIFJFR1JF U1NJT04ocjE4NTI1MSk6IFtHVEtdIHdlYmtpdF93ZWJfY29udGV4dF9zZXRfZGlza19jYWNoZV9k aXJlY3RvcnkoKSBkb2Vzbid0IHdvcmsgd2hlbiB1c2luZyBzaGFyZWQgc2Vjb25kYXJ5IHByb2Nl c3MgbW9kZWwgYWZ0ZXIgcjE4NTI1MQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvTmV0d29y a1Byb2Nlc3MvY2FjaGUvTmV0d29ya0NhY2hlSU9DaGFubmVsU291cC5jcHAgYi9Tb3VyY2UvV2Vi S2l0Mi9OZXR3b3JrUHJvY2Vzcy9jYWNoZS9OZXR3b3JrQ2FjaGVJT0NoYW5uZWxTb3VwLmNwcApp bmRleCA4MDViYzc0Li45NGZlNGY3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9OZXR3b3Jr UHJvY2Vzcy9jYWNoZS9OZXR3b3JrQ2FjaGVJT0NoYW5uZWxTb3VwLmNwcAorKysgYi9Tb3VyY2Uv V2ViS2l0Mi9OZXR3b3JrUHJvY2Vzcy9jYWNoZS9OZXR3b3JrQ2FjaGVJT0NoYW5uZWxTb3VwLmNw cApAQCAtNjksMTUgKzY5LDE1IEBAIFJlZjxJT0NoYW5uZWw+IElPQ2hhbm5lbDo6b3Blbihjb25z dCBTdHJpbmcmIGZpbGVQYXRoLCBJT0NoYW5uZWw6OlR5cGUgdHlwZSkKICAgICByZXR1cm4gYWRv cHRSZWYoKm5ldyBJT0NoYW5uZWwoZmlsZVBhdGgsIHR5cGUpKTsKIH0KIAotc3RhdGljIGlubGlu ZSB2b2lkIHJ1blRhc2tJblF1ZXVlKHN0ZDo6ZnVuY3Rpb248dm9pZCAoKT4gdGFzaywgV29ya1F1 ZXVlKiBxdWV1ZSkKK3N0YXRpYyBpbmxpbmUgdm9pZCBydW5UYXNrSW5RdWV1ZShzdGQ6OmZ1bmN0 aW9uPHZvaWQgKCk+IHRhc2ssIFdvcmtRdWV1ZSogcXVldWUsIGludCBwcmlvcml0eSkKIHsKICAg ICBpZiAocXVldWUpIHsKLSAgICAgICAgcXVldWUtPmRpc3BhdGNoKHRhc2spOworICAgICAgICBx dWV1ZS0+ZGlzcGF0Y2godGFzaywgcHJpb3JpdHkpOwogICAgICAgICByZXR1cm47CiAgICAgfQog CiAgICAgLy8gVXNpbmcgbnVsbHB0ciBhcyBxdWV1ZSBzdWJtaXRzIHRoZSByZXN1bHQgdG8gdGhl IG1haW4gY29udGV4dC4KLSAgICBHTWFpbkxvb3BTb3VyY2U6OnNjaGVkdWxlQW5kRGVsZXRlT25E ZXN0cm95KCJbV2ViS2l0XSBJT0NoYW5uZWwgdGFzayIsIHRhc2spOworICAgIEdNYWluTG9vcFNv dXJjZTo6c2NoZWR1bGVBbmREZWxldGVPbkRlc3Ryb3koIltXZWJLaXRdIElPQ2hhbm5lbCB0YXNr IiwgdGFzaywgcHJpb3JpdHkpOwogfQogCiBzdGF0aWMgdm9pZCBmaWxsRGF0YUZyb21SZWFkQnVm ZmVyKFNvdXBCdWZmZXIqIHJlYWRCdWZmZXIsIHNpemVfdCBzaXplLCBEYXRhJiBkYXRhKQpAQCAt MTYwLDcgKzE2MCw3IEBAIHZvaWQgSU9DaGFubmVsOjpyZWFkKHNpemVfdCBvZmZzZXQsIHNpemVf dCBzaXplLCBXb3JrUXVldWUqIHF1ZXVlLCBzdGQ6OmZ1bmN0aW9uCiAgICAgUmVmUHRyPElPQ2hh bm5lbD4gY2hhbm5lbCh0aGlzKTsKICAgICBydW5UYXNrSW5RdWV1ZShbY2hhbm5lbCwgb2Zmc2V0 LCBzaXplLCBjb21wbGV0aW9uSGFuZGxlcl0gewogICAgICAgICBjaGFubmVsLT5yZWFkKG9mZnNl dCwgc2l6ZSwgY29tcGxldGlvbkhhbmRsZXIpOwotICAgIH0sIHF1ZXVlKTsKKyAgICB9LCBxdWV1 ZSwgR19QUklPUklUWV9ERUZBVUxUKTsKIH0KIAogLy8gRklYTUU6IEl0IHdvdWxkIGJlIGJldHRl ciB0byBkbyB3aXRob3V0IHRoaXMuCkBAIC0yMDksNyArMjA5LDcgQEAgdm9pZCBJT0NoYW5uZWw6 OnJlYWRTeW5jKHNpemVfdCBvZmZzZXQsIHNpemVfdCBzaXplLCBXb3JrUXVldWUqIHF1ZXVlLCBz dGQ6OmZ1bmMKICAgICBydW5UYXNrSW5RdWV1ZShbY2hhbm5lbCwgb2Zmc2V0LCBzaXplLCBjb21w bGV0aW9uSGFuZGxlcl0gewogICAgICAgICBjaGFubmVsLT5yZWFkU3luYyhvZmZzZXQsIHNpemUs IGNvbXBsZXRpb25IYW5kbGVyKTsKICAgICAgICAgZ19jb25kX3NpZ25hbCgmY29uZGl0aW9uKTsK LSAgICB9LCBxdWV1ZSk7CisgICAgfSwgcXVldWUsIEdfUFJJT1JJVFlfREVGQVVMVCk7CiAgICAg Z19jb25kX3dhaXQoJmNvbmRpdGlvbiwgJm11dGV4KTsKIH0KIApAQCAtMjM3LDcgKzIzNyw3IEBA IHN0YXRpYyB2b2lkIG91dHB1dFN0cmVhbVdyaXRlUmVhZHlDYWxsYmFjayhHT3V0cHV0U3RyZWFt KiBzdHJlYW0sIEdBc3luY1Jlc3VsdCoKICAgICBhc3luY0RhdGEtPmJ1ZmZlciA9IGFkb3B0R1Jl Zihzb3VwX2J1ZmZlcl9uZXdfc3ViYnVmZmVyKGFzeW5jRGF0YS0+YnVmZmVyLmdldCgpLCBieXRl c1dyaXR0ZW4sIHBlbmRpbmdCeXRlc1RvV3JpdGUpKTsKICAgICAvLyBVc2UgYSBsb2NhbCB2YXJp YWJsZSBmb3IgdGhlIGRhdGEgYnVmZmVyIHRvIHBhc3MgaXQgdG8gZ19vdXRwdXRfc3RyZWFtX3dy aXRlX2FzeW5jKCksIGJlY2F1c2UgV3JpdGVBc3luY0RhdGEgaXMgcmVsZWFzZWQuCiAgICAgYXV0 byBkYXRhID0gYXN5bmNEYXRhLT5idWZmZXItPmRhdGE7Ci0gICAgZ19vdXRwdXRfc3RyZWFtX3dy aXRlX2FzeW5jKHN0cmVhbSwgZGF0YSwgcGVuZGluZ0J5dGVzVG9Xcml0ZSwgR19QUklPUklUWV9E RUZBVUxULCBudWxscHRyLAorICAgIGdfb3V0cHV0X3N0cmVhbV93cml0ZV9hc3luYyhzdHJlYW0s IGRhdGEsIHBlbmRpbmdCeXRlc1RvV3JpdGUsIEdfUFJJT1JJVFlfREVGQVVMVF9JRExFLCBudWxs cHRyLAogICAgICAgICByZWludGVycHJldF9jYXN0PEdBc3luY1JlYWR5Q2FsbGJhY2s+KG91dHB1 dFN0cmVhbVdyaXRlUmVhZHlDYWxsYmFjayksIGFzeW5jRGF0YS5yZWxlYXNlKCkpOwogfQogCkBA IC0yNTYsNyArMjU2LDcgQEAgdm9pZCBJT0NoYW5uZWw6OndyaXRlKHNpemVfdCBvZmZzZXQsIGNv bnN0IERhdGEmIGRhdGEsIHN0ZDo6ZnVuY3Rpb248dm9pZCAoaW50IGUKIAogICAgIFdyaXRlQXN5 bmNEYXRhKiBhc3luY0RhdGEgPSBuZXcgV3JpdGVBc3luY0RhdGEgeyB0aGlzLCBkYXRhLnNvdXBC dWZmZXIoKSwgY29tcGxldGlvbkhhbmRsZXIgfTsKICAgICAvLyBGSVhNRTogaW1wbGVtZW50IG9m ZnNldC4KLSAgICBnX291dHB1dF9zdHJlYW1fd3JpdGVfYXN5bmMoc3RyZWFtLCBhc3luY0RhdGEt PmJ1ZmZlci0+ZGF0YSwgZGF0YS5zaXplKCksIEdfUFJJT1JJVFlfREVGQVVMVCwgbnVsbHB0ciwK KyAgICBnX291dHB1dF9zdHJlYW1fd3JpdGVfYXN5bmMoc3RyZWFtLCBhc3luY0RhdGEtPmJ1ZmZl ci0+ZGF0YSwgZGF0YS5zaXplKCksIEdfUFJJT1JJVFlfREVGQVVMVF9JRExFLCBudWxscHRyLAog ICAgICAgICByZWludGVycHJldF9jYXN0PEdBc3luY1JlYWR5Q2FsbGJhY2s+KG91dHB1dFN0cmVh bVdyaXRlUmVhZHlDYWxsYmFjayksIGFzeW5jRGF0YSk7CiB9CiAKQEAgLTI2NSw3ICsyNjUsNyBA QCB2b2lkIElPQ2hhbm5lbDo6d3JpdGUoc2l6ZV90IG9mZnNldCwgY29uc3QgRGF0YSYgZGF0YSwg V29ya1F1ZXVlKiBxdWV1ZSwgc3RkOjpmdQogICAgIFJlZlB0cjxJT0NoYW5uZWw+IGNoYW5uZWwo dGhpcyk7CiAgICAgcnVuVGFza0luUXVldWUoW2NoYW5uZWwsIG9mZnNldCwgZGF0YSwgY29tcGxl dGlvbkhhbmRsZXJdIHsKICAgICAgICAgY2hhbm5lbC0+d3JpdGUob2Zmc2V0LCBkYXRhLCBjb21w bGV0aW9uSGFuZGxlcik7Ci0gICAgfSwgcXVldWUpOworICAgIH0sIHF1ZXVlLCBHX1BSSU9SSVRZ X0RFRkFVTFRfSURMRSk7CiB9CiAKIH0gLy8gbmFtZXNwYWNlIE5ldHdvcmtDYWNoZQo=