more bugzilla patches

Talk about code development, features, specific bugzilla bugs, enhancements, patches, and other highly technical things.

Moderator: satrow

Forum rules
Please keep everything here strictly on-topic.
This board is meant for Pale Moon source code development related subjects only like code snippets, patches, specific referenced Bugzilla bugs, mercurial, etc.

This is not for tech support! Please do not post tech support questions in the "Development" board!
Please make sure not to use this board for support questions. Most "bug reports" do not belong in this board and should initially be posted in Community Support or other relevant support boards.

Please keep things on-topic as this forum will be used for reference for Pale Moon development. Expect topics that aren't relevant as such to be moved or deleted.
Post Reply
win7-7
Fanatic
Fanatic
Posts: 176
Joined: 2013-09-16, 15:18
Location: --

more bugzilla patches

Post by win7-7 » 2019-05-21, 18:56


roytam1
Fanatic
Fanatic
Posts: 161
Joined: 2015-03-11, 07:01
Location: Hong Kong

Re: more bugzilla patches

Post by roytam1 » 2019-05-22, 12:24

oh great, wanna try them on my repo.

win7-7
Fanatic
Fanatic
Posts: 176
Joined: 2013-09-16, 15:18
Location: --

Re: more bugzilla patches

Post by win7-7 » 2019-05-22, 12:25

This Mozilla patch also does apply directly.

https://bugzilla.mozilla.org/show_bug.cgi?id=1352734

Question is same for this patch.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 24979
Joined: 2011-08-28, 17:27
Location: 58°2'16"N 14°58'31"E
Contact:

Re: more bugzilla patches

Post by Moonchild » 2019-05-23, 07:12

Instead of just tossing bug numbers our way, can you explain a little what these are for and why they would be needed/wanted?

Also, of note, just because a code patch applies without editing doesn't necessarily mean it doesn't have consequences if other code elsewhere is different, so it's important to understand what a patch is doing and if it is compatible on the whole.
"If you want to build a better world for yourself, you have to be willing to build one for everybody." -- Coyote Osborne
Image

win7-7
Fanatic
Fanatic
Posts: 176
Joined: 2013-09-16, 15:18
Location: --

Re: more bugzilla patches

Post by win7-7 » 2019-05-23, 07:35

There is quick explanation referring Mozilla of what patches do to improve performance.

First patch improves html performance and second dom performance/adds local cache.

https://bugzilla.mozilla.org/show_bug.cgi?id=1351303:

add main thread only cache for nsIAtoms to speed up atomization

make HTML parser to use faster atomization in main thread

https://bugzilla.mozilla.org/show_bug.cgi?id=1352235

NodeInfoManager should use a local cache for recently used nodeinfos to avoid slow hashtable lookups

https://bugzilla.mozilla.org/show_bug.cgi?id=1371508

Simplify the dispatch-to-content region in nsDisplayLayerEventRegions::AddFrame() and AddInactiveScrollPort() if it starts to get large

https://bugzilla.mozilla.org/show_bug.cgi?id=1352734

Another html performance improvement.

Use memcmp and not slower string Equals in nsHtml5Portability::localEqualsBuffer

User avatar
plushkava
Apollo supporter
Apollo supporter
Posts: 41
Joined: 2015-07-31, 04:53
Location: United Kingdom

Re: more bugzilla patches

Post by plushkava » 2019-05-23, 13:42

I had need of an unrelated patch so I monkey-patched these in and spun a Linux x86_64 build, just to see what would happen. Out of the six Mozilla bugs that win7-7 has referenced up until now, #1363423 was already covered. It's not as though I'm formally testing anything here but it builds, at least, with no observable side effects during casual usage.

win7-7
Fanatic
Fanatic
Posts: 176
Joined: 2013-09-16, 15:18
Location: --

Re: more bugzilla patches

Post by win7-7 » 2019-05-23, 17:26

When I tested these patches earlier in New Moon build they seem to give performance wins in artificial dom benchmarks in addition of limited scope testing of certain websites which seems to be faster. There is no negative effects noticeable.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 24979
Joined: 2011-08-28, 17:27
Location: 58°2'16"N 14°58'31"E
Contact:

Re: more bugzilla patches

Post by Moonchild » 2019-05-24, 08:04

Sounds like we want these patches then ;)
Could you please toss up some PRs, one for each bug?

I've created individual issues for the different performance patches and gathered them in the meta Issue #1109 (UXP)

PS, when making code commits for this, PLEASE don't use "patch for {bmo bugnumber}" as the commit message. Describe the code change briefly, and you could mention our issue# if you want to tag it automatically in github. Yes, this is separate from describing the PRs which also has to be done properly. I want to avoid main repohistory entries that refer to a bug system that is not under our control and could go away at any time at which point references to it as code change descriptions become pointless.
"If you want to build a better world for yourself, you have to be willing to build one for everybody." -- Coyote Osborne
Image

win7-7
Fanatic
Fanatic
Posts: 176
Joined: 2013-09-16, 15:18
Location: --

Re: more bugzilla patches

Post by win7-7 » 2019-06-26, 14:08

Attach FrameProperties to each frame instead of using a shared hashtable

https://bugzilla.mozilla.org/show_bug.cgi?id=1365982

For example, in https://perfht.ml/2rv39uP, which is loading the single-page HTML spec, there is 355ms spent in FramePropertyTable methods; around half of this is spent in PLDHashTable methods, i.e. looking up the frame pointer in the hashtable to find its list of properties. What I'd like to consider here is dispensing with the shared hashtable and instead attaching the frame property list directly to nsIFrame.

This depends on some changes into FrameLayerBuilder.cpp and nsTableFrame.cpp and .h files to compile with UXP but works after those changes without noticeable problems during usage.

Also some files were renamed by Mozilla. Changes are into RestyleManager.cpp and RestyleManagerBase.cpp and .h in UXP platform as those correspond to files before Mozilla renaming.

https://bugzilla.mozilla.org/show_bug.cgi?id=1331718 (this has open crash on it which means this isn't good to do making changes to FrameLayerBuilder.cpp needed to main patch/FrameProperties patch to compile)

FrameProperties patch improves performance on:

(benchmark/tests) http://sinz.org/Maze/

All tests there are faster than before.

Also related patches to that which should be included along main patch considering how closely they are related:

https://bugzilla.mozilla.org/show_bug.cgi?id=1378219 (crash fix)

https://bugzilla.mozilla.org/show_bug.cgi?id=1368369 (Reduces RAM usage)

https://bugzilla.mozilla.org/show_bug.cgi?id=1368654 (memory reporter)

Also follow up to main patch:

Look into optimizing out the hashtable lookups from nsContainerFrame::DestroyFrom and nsContainerFrame::SafelyDestroyFrameListProp

https://bugzilla.mozilla.org/show_bug.cgi?id=1367206

This along with main patch and related patches also shows some general improvements in performance.


One more html improvement patch to help html performance also.

https://bugzilla.mozilla.org/show_bug.cgi?id=1347737

An innerHTML setter profile shows about 10% of the time being spent under nsHtml5HtmlAttributes::clear, mostly deleting nsStrings.

Would these patches be wanted to UXP platform?

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 24979
Joined: 2011-08-28, 17:27
Location: 58°2'16"N 14°58'31"E
Contact:

Re: more bugzilla patches

Post by Moonchild » 2019-06-26, 17:06

If you think I'm going to pick this (new thing) up as something trivial you're mistaken. This isn't trivial.
If you want this to land on UXP, I ask that you go ahead and (carefully) port these patches across including any crashes it may cause -- Mozilla's hashtables suck and are extremely prone to crashing, so beware.
"If you want to build a better world for yourself, you have to be willing to build one for everybody." -- Coyote Osborne
Image

win7-7
Fanatic
Fanatic
Posts: 176
Joined: 2013-09-16, 15:18
Location: --

Re: more bugzilla patches

Post by win7-7 » 2019-06-26, 18:20

Moonchild wrote:
2019-06-26, 17:06
If you think I'm going to pick this (new thing) up as something trivial you're mistaken. This isn't trivial.
If you want this to land on UXP, I ask that you go ahead and (carefully) port these patches across including any crashes it may cause -- Mozilla's hashtables suck and are extremely prone to crashing, so beware.
I wasn't asking you to do it since I said that I already have tested these and compiled New Moon/UXP with them and just asked if these would be wanted to improve UXP platform?

Haven't had any stability problem with New Moon build as yet.

User avatar
Isengrim
Board Warrior
Board Warrior
Posts: 1001
Joined: 2015-09-08, 22:54
Location: 127.0.0.1
Contact:

Re: more bugzilla patches

Post by Isengrim » 2019-06-27, 04:39

win7-7 wrote:
2019-06-26, 18:20
I wasn't asking you to do it since I said that I already have tested these and compiled New Moon/UXP with them and just asked if these would be wanted to improve UXP platform?

Haven't had any stability problem with New Moon build as yet.
Definitely focus your testing on UXP if you plan on porting these to UXP. The platform may be significantly different enough from the XP fork that the patches may function differently or expose different issues.
Linux Mint 19.2 Cinnamon (64-bit), Windows 7 (64-bit), Windows 10 build 1803 (64-bit)
"As long as there is someone who will appreciate the work involved in the creation, the effort is time well spent." ~ Tetsuzou Kamadani, Cave Story

roytam1
Fanatic
Fanatic
Posts: 161
Joined: 2015-03-11, 07:01
Location: Hong Kong

Re: more bugzilla patches

Post by roytam1 » 2019-06-27, 06:41

Isengrim wrote:
2019-06-27, 04:39
win7-7 wrote:
2019-06-26, 18:20
I wasn't asking you to do it since I said that I already have tested these and compiled New Moon/UXP with them and just asked if these would be wanted to improve UXP platform?

Haven't had any stability problem with New Moon build as yet.
Definitely focus your testing on UXP if you plan on porting these to UXP. The platform may be significantly different enough from the XP fork that the patches may function differently or expose different issues.
I think he's talking about his unbranded build, not mine. ;)

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 24979
Joined: 2011-08-28, 17:27
Location: 58°2'16"N 14°58'31"E
Contact:

Re: more bugzilla patches

Post by Moonchild » 2019-06-27, 08:38

win7-7 wrote:
2019-06-26, 18:20
I already have tested these and compiled New Moon/UXP with them and just asked if these would be wanted to improve UXP platform?
Oh, I apologize! Excuse me for not reading your post thoroughly and missing that.
Any relevant performance patches/PRs solving bottlenecks are of course welcome!

I do ask that you carefully test these changes against the UXP master branch in that case and if there is anything that touches JS that you please hold off on those until https://github.com/MoonchildProductions/UXP/pull/1142 lands.
"If you want to build a better world for yourself, you have to be willing to build one for everybody." -- Coyote Osborne
Image

roytam1
Fanatic
Fanatic
Posts: 161
Joined: 2015-03-11, 07:01
Location: Hong Kong

Re: more bugzilla patches

Post by roytam1 » 2019-06-28, 13:14

Moderator note: removed useless quote of entire post. Please don't do this. See forum rules, #3. use the "reply" button!
win7-7 wrote:
2019-06-26, 14:08
One more html improvement patch to help html performance also.

https://bugzilla.mozilla.org/show_bug.cgi?id=1347737

An innerHTML setter profile shows about 10% of the time being spent under nsHtml5HtmlAttributes::clear, mostly deleting nsStrings.
what about https://bugzilla.mozilla.org/show_bug.cgi?id=1355441 ?

win7-7
Fanatic
Fanatic
Posts: 176
Joined: 2013-09-16, 15:18
Location: --

Re: more bugzilla patches

Post by win7-7 » 2019-08-04, 22:00

roytam1 wrote:
2019-06-28, 13:14
Moderator note: removed useless quote of entire post. Please don't do this. See forum rules, #3. use the "reply" button!
win7-7 wrote:
2019-06-26, 14:08
One more html improvement patch to help html performance also.

https://bugzilla.mozilla.org/show_bug.cgi?id=1347737

An innerHTML setter profile shows about 10% of the time being spent under nsHtml5HtmlAttributes::clear, mostly deleting nsStrings.
what about https://bugzilla.mozilla.org/show_bug.cgi?id=1355441 ?
At lot earlier time I tried to apply https://bugzilla.mozilla.org/show_bug.cgi?id=1355441 based to your porting of it however it showed performance regression in real website and some performance regression on tests when compared to without it (https://bugzilla.mozilla.org/show_bug.cgi?id=934607). It also showed no improvements on meta-bug test case (https://bugzilla.mozilla.org/show_bug.cgi?id=1347525).




Moonchild:

One performance improvement to DOM could be done to UXP/ using AsyncOpen2. AsyncOpen2 is already used in elsewhere in UXP. Is this and related patches wanted changes for UXP?

I have compiled New Moon and tested 1267075, 1373780 (part 2 and 3) and 1445670 in it.

https://bugzilla.mozilla.org/show_bug.cgi?id=1267075

Shows better performance in this DOM test http://www.hixie.ch/tests/adhoc/perf/do ... d/005.html

and related patches:

https://bugzilla.mozilla.org/show_bug.cgi?id=1373780

part 2 and part 3 of it.

https://bugzilla.mozilla.org/show_bug.cgi?id=1445670

https://bugzilla.mozilla.org/show_bug.cgi?id=1411473 (this patch does not seem to apply to UXP since nsGenericHTMLElement::NodeInfoChanged(aOldDoc); is not present in UXP file)

roytam1
Fanatic
Fanatic
Posts: 161
Joined: 2015-03-11, 07:01
Location: Hong Kong

Re: more bugzilla patches

Post by roytam1 » 2019-08-08, 01:28

win7-7 wrote:
2019-08-04, 22:00
roytam1 wrote:
2019-06-28, 13:14
Moderator note: removed useless quote of entire post. Please don't do this. See forum rules, #3. use the "reply" button!
win7-7 wrote:
2019-06-26, 14:08
One more html improvement patch to help html performance also.

https://bugzilla.mozilla.org/show_bug.cgi?id=1347737

An innerHTML setter profile shows about 10% of the time being spent under nsHtml5HtmlAttributes::clear, mostly deleting nsStrings.
what about https://bugzilla.mozilla.org/show_bug.cgi?id=1355441 ?
At lot earlier time I tried to apply https://bugzilla.mozilla.org/show_bug.cgi?id=1355441 based to your porting of it however it showed performance regression in real website and some performance regression on tests when compared to without it (https://bugzilla.mozilla.org/show_bug.cgi?id=934607). It also showed no improvements on meta-bug test case (https://bugzilla.mozilla.org/show_bug.cgi?id=1347525).
but it seems that memory usage is better (less memory fragments) after applying that patch.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 24979
Joined: 2011-08-28, 17:27
Location: 58°2'16"N 14°58'31"E
Contact:

Re: more bugzilla patches

Post by Moonchild » 2019-08-08, 08:38

AsyncOpen2 tends to perform better than AsyncOpen in general, so if you want to make a PR that ports more stuff to AsyncOpen2, then I'm all for it.
"If you want to build a better world for yourself, you have to be willing to build one for everybody." -- Coyote Osborne
Image

Post Reply