Re: Review Request 127632: Prioritize correct extension per theme

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/
---

(Updated April 27, 2016, 12:36 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Andreas Kainz and Christoph Feck.


Changes
---

Submitted with commit 536d707981ada478b583f91f31a9dca741881a36 by Aleix Pol to 
branch master.


Repository: kiconthemes


Description
---

Usually themes will have 1 kind of extension (2 tops) it doesn't make sense to 
bindly use an extensions vector that is statically defined.

Prioritizes the extensions that work, so it will find the icons sooner.


Diffs
-

  src/kiconengine.h 5b9badb 
  src/kiconloader.cpp 37528ad 
  src/kicontheme.h 3190665 
  src/kicontheme.cpp e7e003b 

Diff: https://git.reviewboard.kde.org/r/127632/diff/


Testing
---

Builds and tests still pass.
Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
almost as good as the RR #127236, but with a much smaller memory impact, since 
we're not caching all the icon names (which was specially bad as it was 
per-process).
```
$ strace kwrite |& grep ENOENT | wc -l
4699
$ strace kwrite |& grep ENOENT | wc -l
2119
```


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/#review94881
---


Ship it!




Ship It!

- Albert Astals Cid


On April 26, 2016, 8:51 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127632/
> ---
> 
> (Updated April 26, 2016, 8:51 p.m.)
> 
> 
> Review request for KDE Frameworks, Andreas Kainz and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Usually themes will have 1 kind of extension (2 tops) it doesn't make sense 
> to bindly use an extensions vector that is statically defined.
> 
> Prioritizes the extensions that work, so it will find the icons sooner.
> 
> 
> Diffs
> -
> 
>   src/kiconengine.h 5b9badb 
>   src/kiconloader.cpp 37528ad 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp e7e003b 
> 
> Diff: https://git.reviewboard.kde.org/r/127632/diff/
> 
> 
> Testing
> ---
> 
> Builds and tests still pass.
> Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
> almost as good as the RR #127236, but with a much smaller memory impact, 
> since we're not caching all the icon names (which was specially bad as it was 
> per-process).
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 4699
> $ strace kwrite |& grep ENOENT | wc -l
> 2119
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/
---

(Updated April 26, 2016, 10:51 p.m.)


Review request for KDE Frameworks, Andreas Kainz and Christoph Feck.


Changes
---

Address Albert issues.


Repository: kiconthemes


Description
---

Usually themes will have 1 kind of extension (2 tops) it doesn't make sense to 
bindly use an extensions vector that is statically defined.

Prioritizes the extensions that work, so it will find the icons sooner.


Diffs (updated)
-

  src/kiconengine.h 5b9badb 
  src/kiconloader.cpp 37528ad 
  src/kicontheme.h 3190665 
  src/kicontheme.cpp e7e003b 

Diff: https://git.reviewboard.kde.org/r/127632/diff/


Testing
---

Builds and tests still pass.
Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
almost as good as the RR #127236, but with a much smaller memory impact, since 
we're not caching all the icon names (which was specially bad as it was 
per-process).
```
$ strace kwrite |& grep ENOENT | wc -l
4699
$ strace kwrite |& grep ENOENT | wc -l
2119
```


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Aleix Pol Gonzalez]

> On April 26, 2016, 10:02 p.m., Andreas Kainz wrote:
> > Sorry someone add my but I can't follow the discussion. I don't know what 
> > this is doing. sorry.

I wanted to make you aware of this change:
`Needs patching in Breeze & Oxygen: https://paste.kde.org/py5kqvppv`


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/#review94875
---


On April 26, 2016, 10:51 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127632/
> ---
> 
> (Updated April 26, 2016, 10:51 p.m.)
> 
> 
> Review request for KDE Frameworks, Andreas Kainz and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Usually themes will have 1 kind of extension (2 tops) it doesn't make sense 
> to bindly use an extensions vector that is statically defined.
> 
> Prioritizes the extensions that work, so it will find the icons sooner.
> 
> 
> Diffs
> -
> 
>   src/kiconengine.h 5b9badb 
>   src/kiconloader.cpp 37528ad 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp e7e003b 
> 
> Diff: https://git.reviewboard.kde.org/r/127632/diff/
> 
> 
> Testing
> ---
> 
> Builds and tests still pass.
> Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
> almost as good as the RR #127236, but with a much smaller memory impact, 
> since we're not caching all the icon names (which was specially bad as it was 
> per-process).
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 4699
> $ strace kwrite |& grep ENOENT | wc -l
> 2119
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Andreas Kainz]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/#review94875
---



Sorry someone add my but I can't follow the discussion. I don't know what this 
is doing. sorry.

- Andreas Kainz


On April 26, 2016, 7:12 nachm., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127632/
> ---
> 
> (Updated April 26, 2016, 7:12 nachm.)
> 
> 
> Review request for KDE Frameworks, Andreas Kainz and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Usually themes will have 1 kind of extension (2 tops) it doesn't make sense 
> to bindly use an extensions vector that is statically defined.
> 
> Prioritizes the extensions that work, so it will find the icons sooner.
> 
> 
> Diffs
> -
> 
>   src/kiconengine.h 5b9badb 
>   src/kiconloader.cpp 37528ad 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp e7e003b 
> 
> Diff: https://git.reviewboard.kde.org/r/127632/diff/
> 
> 
> Testing
> ---
> 
> Builds and tests still pass.
> Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
> almost as good as the RR #127236, but with a much smaller memory impact, 
> since we're not caching all the icon names (which was specially bad as it was 
> per-process).
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 4699
> $ strace kwrite |& grep ENOENT | wc -l
> 2119
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/#review94873
---




src/kicontheme.h (line 173)


@since missing



src/kicontheme.cpp (line 407)


constBegin + constEnd + const_iterator (or auto?)


- Albert Astals Cid


On April 26, 2016, 7:12 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127632/
> ---
> 
> (Updated April 26, 2016, 7:12 p.m.)
> 
> 
> Review request for KDE Frameworks, Andreas Kainz and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Usually themes will have 1 kind of extension (2 tops) it doesn't make sense 
> to bindly use an extensions vector that is statically defined.
> 
> Prioritizes the extensions that work, so it will find the icons sooner.
> 
> 
> Diffs
> -
> 
>   src/kiconengine.h 5b9badb 
>   src/kiconloader.cpp 37528ad 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp e7e003b 
> 
> Diff: https://git.reviewboard.kde.org/r/127632/diff/
> 
> 
> Testing
> ---
> 
> Builds and tests still pass.
> Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
> almost as good as the RR #127236, but with a much smaller memory impact, 
> since we're not caching all the icon names (which was specially bad as it was 
> per-process).
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 4699
> $ strace kwrite |& grep ENOENT | wc -l
> 2119
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/
---

(Updated April 26, 2016, 9:12 p.m.)


Review request for KDE Frameworks, Andreas Kainz and Christoph Feck.


Repository: kiconthemes


Description
---

Usually themes will have 1 kind of extension (2 tops) it doesn't make sense to 
bindly use an extensions vector that is statically defined.

Prioritizes the extensions that work, so it will find the icons sooner.


Diffs (updated)
-

  src/kiconengine.h 5b9badb 
  src/kiconloader.cpp 37528ad 
  src/kicontheme.h 3190665 
  src/kicontheme.cpp e7e003b 

Diff: https://git.reviewboard.kde.org/r/127632/diff/


Testing
---

Builds and tests still pass.
Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
almost as good as the RR #127236, but with a much smaller memory impact, since 
we're not caching all the icon names (which was specially bad as it was 
per-process).
```
$ strace kwrite |& grep ENOENT | wc -l
4699
$ strace kwrite |& grep ENOENT | wc -l
2119
```


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/
---

(Updated April 26, 2016, 5:42 p.m.)


Review request for KDE Frameworks, Andreas Kainz and Christoph Feck.


Changes
---

Let themes specify their own extensions to be looked up.

The strace test results are quite similar and doesn't break standards.

Needs patching in Breeze & Oxygen though: https://paste.kde.org/py5kqvppv


Repository: kiconthemes


Description
---

Usually themes will have 1 kind of extension (2 tops) it doesn't make sense to 
bindly use an extensions vector that is statically defined.

Prioritizes the extensions that work, so it will find the icons sooner.


Diffs (updated)
-

  src/kiconengine.h 5b9badb 
  src/kiconloader.cpp 37528ad 
  src/kicontheme.h 3190665 
  src/kicontheme.cpp e7e003b 

Diff: https://git.reviewboard.kde.org/r/127632/diff/


Testing
---

Builds and tests still pass.
Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
almost as good as the RR #127236, but with a much smaller memory impact, since 
we're not caching all the icon names (which was specially bad as it was 
per-process).
```
$ strace kwrite |& grep ENOENT | wc -l
4699
$ strace kwrite |& grep ENOENT | wc -l
2119
```


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Anthony Fieroni]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/#review94851
---



+1

without patch
strace kate |& grep ENOENT | wc -l
9369
strace dolphin |& grep ENOENT | wc -l
9603

with patch
strace kate |& grep ENOENT | wc -l
4021
strace dolphin |& grep ENOENT | wc -l
5278

- Anthony Fieroni


On Април 12, 2016, 2:17 преди обяд, Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127632/
> ---
> 
> (Updated Април 12, 2016, 2:17 преди обяд)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Usually themes will have 1 kind of extension (2 tops) it doesn't make sense 
> to bindly use an extensions vector that is statically defined.
> 
> Prioritizes the extensions that work, so it will find the icons sooner.
> 
> 
> Diffs
> -
> 
>   src/kiconloader.cpp 37528ad 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp e7e003b 
> 
> Diff: https://git.reviewboard.kde.org/r/127632/diff/
> 
> 
> Testing
> ---
> 
> Builds and tests still pass.
> Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
> almost as good as the RR #127236, but with a much smaller memory impact, 
> since we're not caching all the icon names (which was specially bad as it was 
> per-process).
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 4699
> $ strace kwrite |& grep ENOENT | wc -l
> 2119
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Nick Shaforostoff]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/#review94759
---


Ship it!




i think adhering spec here is not very important.

we can adopt this change now, mention this somewhere in docs (print a qWarning 
saying that the checking order has changed?), and introduce X-KDE-Extensions 
later (which would only require a slght adjustment of iconPathByName() method).

- Nick Shaforostoff


On April 11, 2016, 11:17 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127632/
> ---
> 
> (Updated April 11, 2016, 11:17 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Usually themes will have 1 kind of extension (2 tops) it doesn't make sense 
> to bindly use an extensions vector that is statically defined.
> 
> Prioritizes the extensions that work, so it will find the icons sooner.
> 
> 
> Diffs
> -
> 
>   src/kiconloader.cpp 37528ad 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp e7e003b 
> 
> Diff: https://git.reviewboard.kde.org/r/127632/diff/
> 
> 
> Testing
> ---
> 
> Builds and tests still pass.
> Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
> almost as good as the RR #127236, but with a much smaller memory impact, 
> since we're not caching all the icon names (which was specially bad as it was 
> per-process).
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 4699
> $ strace kwrite |& grep ENOENT | wc -l
> 2119
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Albert Astals Cid]

> On April 11, 2016, 10:44 p.m., Albert Astals Cid wrote:
> > I'm unconvinced, actually the spec mentions order to be "Changed search 
> > order to png, svg, xpm"
> 
> Albert Astals Cid wrote:
> 
> https://specifications.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html
> 
> Aleix Pol Gonzalez wrote:
> I see, I didn't know that. Still, I believe we should adopt the patch for 
> better performance.
> 
> This will be a problem only if there's some icons that are in svg (or 
> xpm) and not in png, but the rest do have png.

Honestly i'd prefer adding an extension to the .desktop of the icon theme 
called X-KDE-Extensions that lists the order in which extensions are checked 
and that for our themes we just set to svg. But if other people think adhering 
to the spec is not important i can live with it too.


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/#review94547
---


On April 11, 2016, 11:17 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127632/
> ---
> 
> (Updated April 11, 2016, 11:17 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Usually themes will have 1 kind of extension (2 tops) it doesn't make sense 
> to bindly use an extensions vector that is statically defined.
> 
> Prioritizes the extensions that work, so it will find the icons sooner.
> 
> 
> Diffs
> -
> 
>   src/kiconloader.cpp 37528ad 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp e7e003b 
> 
> Diff: https://git.reviewboard.kde.org/r/127632/diff/
> 
> 
> Testing
> ---
> 
> Builds and tests still pass.
> Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
> almost as good as the RR #127236, but with a much smaller memory impact, 
> since we're not caching all the icon names (which was specially bad as it was 
> per-process).
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 4699
> $ strace kwrite |& grep ENOENT | wc -l
> 2119
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/
---

(Updated April 12, 2016, 1:17 a.m.)


Review request for KDE Frameworks and Christoph Feck.


Changes
---

Remove wrong change, pointed out by David E.


Repository: kiconthemes


Description
---

Usually themes will have 1 kind of extension (2 tops) it doesn't make sense to 
bindly use an extensions vector that is statically defined.

Prioritizes the extensions that work, so it will find the icons sooner.


Diffs (updated)
-

  src/kiconloader.cpp 37528ad 
  src/kicontheme.h 3190665 
  src/kicontheme.cpp e7e003b 

Diff: https://git.reviewboard.kde.org/r/127632/diff/


Testing
---

Builds and tests still pass.
Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
almost as good as the RR #127236, but with a much smaller memory impact, since 
we're not caching all the icon names (which was specially bad as it was 
per-process).
```
$ strace kwrite |& grep ENOENT | wc -l
4699
$ strace kwrite |& grep ENOENT | wc -l
2119
```


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Aleix Pol Gonzalez]

> On April 12, 2016, 12:44 a.m., Albert Astals Cid wrote:
> > I'm unconvinced, actually the spec mentions order to be "Changed search 
> > order to png, svg, xpm"
> 
> Albert Astals Cid wrote:
> 
> https://specifications.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html

I see, I didn't know that. Still, I believe we should adopt the patch for 
better performance.

This will be a problem only if there's some icons that are in svg (or xpm) and 
not in png, but the rest do have png.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/#review94547
---


On April 11, 2016, 1:05 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127632/
> ---
> 
> (Updated April 11, 2016, 1:05 a.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Usually themes will have 1 kind of extension (2 tops) it doesn't make sense 
> to bindly use an extensions vector that is statically defined.
> 
> Prioritizes the extensions that work, so it will find the icons sooner.
> 
> 
> Diffs
> -
> 
>   src/kiconloader.cpp 37528ad 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp e7e003b 
> 
> Diff: https://git.reviewboard.kde.org/r/127632/diff/
> 
> 
> Testing
> ---
> 
> Builds and tests still pass.
> Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
> almost as good as the RR #127236, but with a much smaller memory impact, 
> since we're not caching all the icon names (which was specially bad as it was 
> per-process).
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 4699
> $ strace kwrite |& grep ENOENT | wc -l
> 2119
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Albert Astals Cid]

> On April 11, 2016, 10:44 p.m., Albert Astals Cid wrote:
> > I'm unconvinced, actually the spec mentions order to be "Changed search 
> > order to png, svg, xpm"

https://specifications.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/#review94547
---


On April 10, 2016, 11:05 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127632/
> ---
> 
> (Updated April 10, 2016, 11:05 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Usually themes will have 1 kind of extension (2 tops) it doesn't make sense 
> to bindly use an extensions vector that is statically defined.
> 
> Prioritizes the extensions that work, so it will find the icons sooner.
> 
> 
> Diffs
> -
> 
>   src/kiconloader.cpp 37528ad 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp e7e003b 
> 
> Diff: https://git.reviewboard.kde.org/r/127632/diff/
> 
> 
> Testing
> ---
> 
> Builds and tests still pass.
> Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
> almost as good as the RR #127236, but with a much smaller memory impact, 
> since we're not caching all the icon names (which was specially bad as it was 
> per-process).
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 4699
> $ strace kwrite |& grep ENOENT | wc -l
> 2119
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/#review94547
---



I'm unconvinced, actually the spec mentions order to be "Changed search order 
to png, svg, xpm"

- Albert Astals Cid


On April 10, 2016, 11:05 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127632/
> ---
> 
> (Updated April 10, 2016, 11:05 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Usually themes will have 1 kind of extension (2 tops) it doesn't make sense 
> to bindly use an extensions vector that is statically defined.
> 
> Prioritizes the extensions that work, so it will find the icons sooner.
> 
> 
> Diffs
> -
> 
>   src/kiconloader.cpp 37528ad 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp e7e003b 
> 
> Diff: https://git.reviewboard.kde.org/r/127632/diff/
> 
> 
> Testing
> ---
> 
> Builds and tests still pass.
> Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
> almost as good as the RR #127236, but with a much smaller memory impact, 
> since we're not caching all the icon names (which was specially bad as it was 
> per-process).
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 4699
> $ strace kwrite |& grep ENOENT | wc -l
> 2119
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[David Edmundson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/#review94546
---


Ship it!




Ship It!

- David Edmundson


On April 10, 2016, 11:05 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127632/
> ---
> 
> (Updated April 10, 2016, 11:05 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Usually themes will have 1 kind of extension (2 tops) it doesn't make sense 
> to bindly use an extensions vector that is statically defined.
> 
> Prioritizes the extensions that work, so it will find the icons sooner.
> 
> 
> Diffs
> -
> 
>   src/kiconloader.cpp 37528ad 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp e7e003b 
> 
> Diff: https://git.reviewboard.kde.org/r/127632/diff/
> 
> 
> Testing
> ---
> 
> Builds and tests still pass.
> Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
> almost as good as the RR #127236, but with a much smaller memory impact, 
> since we're not caching all the icon names (which was specially bad as it was 
> per-process).
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 4699
> $ strace kwrite |& grep ENOENT | wc -l
> 2119
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127632: Prioritize correct extension per theme

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127632/
---

(Updated April 11, 2016, 1:05 a.m.)


Review request for KDE Frameworks and Christoph Feck.


Changes
---

Don't change the extensions vector if it was already in place.


Repository: kiconthemes


Description
---

Usually themes will have 1 kind of extension (2 tops) it doesn't make sense to 
bindly use an extensions vector that is statically defined.

Prioritizes the extensions that work, so it will find the icons sooner.


Diffs (updated)
-

  src/kiconloader.cpp 37528ad 
  src/kicontheme.h 3190665 
  src/kicontheme.cpp e7e003b 

Diff: https://git.reviewboard.kde.org/r/127632/diff/


Testing
---

Builds and tests still pass.
Also reducess faulty disk accesses to its 45%. Note that by this metric, it's 
almost as good as the RR #127236, but with a much smaller memory impact, since 
we're not caching all the icon names (which was specially bad as it was 
per-process).
```
$ strace kwrite |& grep ENOENT | wc -l
4699
$ strace kwrite |& grep ENOENT | wc -l
2119
```


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel