Update to 61 - Githubissues

xsmile commented 7 years ago

Updated existing patches only, didn't look for any new unwanted code.

perfect7gentleman commented 7 years ago

built Chromium with those patches in Gentoo

gcarq commented 7 years ago

Thanks for the PR

Unfortunately I can't compile chromium right now because I got a (faulty?) Ryzen 7 CPU and it causes random segfaults if under heavy load. This really sucks, but I hope it will be sorted out quickly

nyancat18 commented 7 years ago

you could make this

1 disable SMT or the uOP cache via the CMOS setup of your mainboard

https://community.amd.com/thread/215773

2 if this no work, please enable LLC in BIOS (it disables a default underlock under heavy load)

3 In the Asus BIOS there is an option called called OPCache Control. Disabling this may resolve this issue.

4 disable ASLR

nyancat18 commented 7 years ago

5 if all fails, disable OPCACHE + ASLR, enable LLC

gcarq commented 7 years ago

Thanks for the recommendations, I already tried the suggestions from this thread and at first look they seem to work, but the system is still unstable (it's just harder to trigger), I'm not comfortable building software under this conditions. The good news is, I'm in contact with AMD hopefully they will provide a microcode update or CPU replacement

nyancat18 commented 7 years ago

ok

nyancat18 commented 7 years ago

did you tried make more things? 1 disable multithreading (8 cores/16 threads)

if all 16 treads are used .... and gcc believes that you've 16 INSTEAD OF 8 misfortunes will happen

2 give it more cooling (without heat the default underlock its avoided)

3 change flags for make it use just 8 (something like make -j8) SLOWER BUT MAY WORK

Eloston commented 7 years ago

@xsmile Out of curiosity, why are you setting a custom toolchain to build with Clang? Wouldn't it be easier to set is_clang=true and clang_base_path GN arguments instead?

Also, are there any side effects to using gcc_toolchain GN template instead of the clang_toolchain template? (example usage would be build/toolchain/linux/BUILD.gn). Also, would this break cross-compilation?

xsmile commented 7 years ago

@Eloston: I tried it first and it would have been the easier way but GN ignores environment variables like CFLAGS and makes it imposssible to edit compiler flags without patching the source. Gentoo has a workaround for this issue and reads/sets environment variables with a custom BUILD.gn before invoking the official gcc_toolchain template. Google has already changed this behavior by creating a new unbundle template but it will take a while before the patch arrives in stable builds, see https://chromium-review.googlesource.com/c/575984.

According to the comment lines, clang_toolchain seems to be a wrapper for gcc_toolchain and is intended to be used with the Chromium version of Clang. It doesn't contain any important settings, as far as I can tell.

Cross-compilation probably needs some changes to work but I have no experience with it. Gentoo's ebuild file handles it at [1] and sets different toolchains for host_toolchain and custom_toolchain.

https://gitweb.gentoo.org/repo/gentoo.git/tree/www-client/chromium/chromium-60.0.3112.78.ebuild#n484

Eloston commented 7 years ago

@xsmile Oh I see. Debian solved that issue by declaring additional arguments for adding compiler flags. Thanks for the info.

nyancat18 commented 7 years ago

@xsmile @perfect7gentleman

sorry for an offtopic question

but what CPU do you use?

i want build inox

xsmile commented 7 years ago

@nyancat18: I use an Intel i7-3610QM. I recommend ccache if you want to compile it more than once, it will greatly speed up subsequent builds. Expect long compile times.

Another off-topic question - Did anyone test link-time optimizations (LTO) and control flow integrity (CFI) yet? Google uses both for official Linux builds.

nyancat18 commented 7 years ago

thanks

@xsmile what Buildtime do you've (it says i5 3550-16 ram = 3 hours)

perfect7gentleman commented 7 years ago

@nyancat18 , i7 Haswell

Eloston commented 7 years ago

@xsmile If you're going to disable MDNS, you might as well disable Service Discovery via enable_service_discovery=false. It doesn't require any additional patching.

xsmile commented 7 years ago

@Eloson: Only for OS X due to enable_service_discovery = enable_mdns || is_mac. Might be better to explicitly disable it as that behavior can change.

Regarding the profiling patch from 9dd2bda - I couldn't find a way to completely disable it without editing the source. Even --enable-profiling=0 (seems to be a bug as =1 activates the flag and =0 does nothing) doesn't disable it completely.

I did some tests with CFI and LTO by the way. LTO is useful only in combination with ThinLTO as it greatly decreases memory requirements. The build takes a bit longer to finish but that's fine. Subsequent builds are affected but can be sped up by using cache. CFI seems like an interesting feature but would be a flag for final builds or for those with a more powerful system as it increases build time even more and additionally code size.

@nyancat18: I can't give you an exact number but definitely more than 3 hours, however only a couple of minutes with cache.

nyancat18 commented 7 years ago

@xsmile
thanks

@perfect7gentleman do you need more than 3 hours or 1-2 are file

DragoonAethis commented 7 years ago

@nyancat18 If you want to, just try it on your system.

Download the latest repo (xsmile's), unpack it somewhere (preferrably on an SSD), make sure you've got 20GBs free, open the terminal, cd to the directory with unpacked PKGBUILD and all the other files, then type something like date > started && makepkg -s && date > finished - it'll take a while, and you'll get the installable .pkg.tar.* package and started/finished files that will tell you how long exactly it took on your system.

It will take a long while, this is one of the largest open source projects in the world. Start it and come back in a few hours (after work/sleep/school/etc).

Eloston commented 7 years ago

@xsmile

Only for OS X due to enable_service_discovery = enable_mdns || is_mac. Might be better to explicitly disable it as that behavior can change.

That's true, but I thought Inox was exclusive to desktop Linux? I've only seen people using it on Arch.

Regarding profiling, what's the reason for disabling it with the patch? I'm curious to know the impacts it has on performance or efficiency.

Regarding LTO and CFI, have you noticed any significant impacts on browsing after enabling it?

xsmile commented 7 years ago

@Eloston: Now I'm confused :) enable_service_discovery is implicitly disabled on Linux if enable_mdns is false which should be enough for Inox.

Disabling profiling shouldn't make too much difference, this is also described in the official developer docs [1]. I would like to avoid running unnecessary things in the background and as the flag to disable it does not work, I figured this is the way to go.

I can only confirm that LTO reduces the binary size by several MB. According to [2] and [3] it speeds up Chromium but I don't know how to benchmark the browser. CFI increases code size and decreases performance, see [3]. By using both you should have increased security and hardly any changes in performance. By enabling CFI, GN will also append compiler flags for LTO/ThinLTO as it is a requirement.

nyancat18 commented 7 years ago

@DragoonAethis 4 hours?

a good eta (average cpu 16 ram)

Eloston commented 7 years ago

@xsmile Hmm, I'm not exactly sure what point I was trying to make when writing that. But like you said, it should probably be explicitly set just to be safe for future versions.

Regarding benchmarking the browser, there is some info about profiling in the docs (as you might already know), but I'm not exactly sure what we should be testing to measure the differences CFI and LTO make.

DragoonAethis commented 7 years ago

@Eloston How about these benchmarks? This is AFAIK the same set of tests upstream uses to continuously test Chrome here.

@nyancat18 Expect 4+ hours or longer.

Eloston commented 7 years ago

@DragoonAethis That looks like it would be sufficient, but I don't have a setup to test it (nor a strong incentive to do so)

perfect7gentleman commented 7 years ago

@nyancat18, about 2 hours

nyancat18 commented 7 years ago

@xsmile @perfect7gentleman @DragoonAethis @Eloston @gcarq

A had somebody a special setup? ssd / tmpfs build?

B with 16 ram could i build chromium with tmpfs? or i must wait and build with HDD/SSD

perfect7gentleman commented 7 years ago

@nyancat18 , you can

nyancat18 commented 7 years ago

thanks

nyancat18 commented 7 years ago

@xsmile can i ask 3 questions?

A your master branch its stable (build without surprises) ?

B why the new dependency (openh264)

C 16 ram .. its enught?

nyancat18 commented 7 years ago

sorry are 4 questions

D the new DDK patch .. its mandatory? i prefer searx/startpage lol

perfect7gentleman commented 7 years ago

@xsmile , built chromium-61 with your latest patches

xsmile commented 7 years ago

@nyancat18, I think you have figured it out already.

@perfect7gentleman, you can include the safe-browsing patches again.

nyancat18 commented 7 years ago

@perfect7gentleman i got this https://u.teknik.io/g0Idr

xsmile commented 7 years ago

@DragoonAethis and @Eloston, I ran some benchmarks as outlined in https://www.chromium.org/developers/design-documents/rendering-benchmarks but this is not something I would like to repeat in future. In theory you just choose a benchmark and run it with run_benchmark on a pre-recorded page set. However you need to consider several things first:

the telemetry toolkit needs to be patched to find Inox due to its custom name
Google did not commit page sets to avoid legal issues and you need to record your own ones
recording specific pages does not work if they require login data because of outdated login scripts
a custom benchmark needs to be created and maintained containing only recordable pages
multiple benchmark runs are required to get meaningful results because of a high score variance

I can provide more details if you really want to know how to set it up.

By the way, woolyss has recently enabled ThinLTO for his Windows builds and the comment section on his page contains some benchmark results.

Eloston commented 7 years ago

Hmm interesting. Based on what I've read, the change is not significant but it is noticeable. Thanks for the info @xsmile.

gcarq commented 7 years ago

It's been a long time since the latest release, sorry for t hat!. Thanks @xsmile for all the PR's it means a lot, I would love to invite you to the contributors If you want (and if you have the time ofc), so things can be merged in proper time. Nevertheless I'm back from holidays and got a new CPU (didn't test it yet though), so you can expect a 61.X release the next days

nyancat18 commented 7 years ago

@gcarq wha cpu did you bought (now)

a new ryzen (but after july = no segfaults)
an old FX (i build it with a FX, 3.4 hours without ccache)
an intel i5/i7

xsmile commented 7 years ago

@gcarq, nice to see you back.

I would like to contribute to Inox but I can't make any promise about my activity.

The current PKGBUILD introduces several changes and it would be great if you can review/confirm them. I will commit them afterwards.

build option for Clang using llvm tools and lld linker, disabled by default. I'm not sure how to handle the default here as Arch Linux uses GCC but Google prefers Clang. Clang seems to require less maintenance (no GCC patches) and I used it without visible issues for some time already
previously custom compiler flags (or those set my /etc/makepkg.conf) were ignored - now they are not
VA-API for Linux is always allowed but must be explicitly enabled in chrome://flags. If it cannot be used for some reason, Chromium falls back to software decoding

Apart from that I had some fun with partially disabling metrics and creating other minor patches. You can see the results in the develop branch of my fork. The relevant patches start at 9000-.

gcarq commented 7 years ago

@nyancat18 It's still a Ryzen 7 1800x. I was in contact with AMD support and they sent me a replacement product (until now it works as expected, but I didn't compile chromium yet).

@xsmile I would also prefer clang over gcc. gcc feels more error-prone these days and patching around the compiler just feels wrong. Thanks a lot, I'm looking forward to merge them :-)

I'm also creating a develop branch for this repo, feel free to push your changes there (I've sent you an invite), in future I would like to introduce the git flow workflow where master is always at the commit from the latest release.

xsmile commented 7 years ago

@gcarq, I pushed updated files for the current Chromium version to the develop branch and created separate branches for the metrics and profiler patches.

Going to remove my fork as it is not needed anymore.

nyancat18 commented 7 years ago

@xsmile could you make a new branch (with your exfork) ?

with ALL blocked? (metric, prolifier, tumbs and avatar)

gcarq commented 7 years ago

@xsmile Thanks a lot, I'm going to test it. Is my assumption correct that this PR can be closed?

xsmile commented 7 years ago

@nyancat18: This is not up to me.

@gcarq: Yes, closing it now. I look forward to an official release.

nyancat18 commented 6 years ago

@DragoonAethis @perfect7gentleman @Eloston @gcarq

i'm making some edgy modifications for inox

and i want report something, the gn flag 'enable_service_discovery=false' defeats build

https://p.teknik.io/Raw/olbFC

afik because its redundant (linux had this disabled and if you repeat it its redundant)

perfect7gentleman commented 6 years ago

@nyancat18 , me personally removed it in my ebuild

ghost commented 6 years ago

afik because its redundant (linux had this disabled and if you repeat it its redundant)

If it's redundant, then why adding the same flag would cause an error?! Shouldn't it just be ignored? :thinking:

https://p.teknik.io/Raw/olbFC

The linker error you're seeing is related to enable_mdns=false. Which is buggy on 61. See https://github.com/gcarq/inox-patchset/blob/develop/PKGBUILD#L236

gcarq / inox-patchset

Update to 61 #90