Re: [Okular-devel] Review Request 109119: Txt backend: Drop detectEncoding method and just feed more data to KEncodingProber

[Azat Khuzhin]

> On Feb. 27, 2013, 10:43 p.m., Albert Astals Cid wrote:
> > Code looks, ok, but now the big question, is this really an improvement? I 
> > guess the only improvement would be speed-wise and not 
> > encoding-detection-wise. Have you opened really big files and noticed it is 
> > faster? Otherwise maybe there is no really need to change the existing code 
> > if it works as well?
> 
> Azat Khuzhin wrote:
> Yes it is only speed-wise improvement, as I already told.
> > Description:
> > Don't to extra job, when encoding is detected just return encoding.
> 
> For big files there is improvement, but this files is quite big (~900 000 
>  lines long), if we oriented on books
> Just two times faster for such number of lines.
> 
> Maybe there is no need in this patch, but for me it more appropriate way.
> 
> Albert Astals Cid wrote:
> Ok, i'm going to commit it. I have created a file with 1.7 million lines 
> and the old code takes 270ms in running detectEncoding over that file and the 
> new one takes 0ms, but still there is something out there taking a 
> mch longer time so it's optimizing the wrong part of the code imo.

Yes, it's true, it opens huge documents, too long.
I'v already profile this using some tools, but can't find how can I make it 
faster, when I would have more time, i will try once more.

Maybe threaded render will do this faster.


- Azat


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109119/#review28231
---


On Feb. 25, 2013, 8:15 p.m., Azat Khuzhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109119/
> ---
> 
> (Updated Feb. 25, 2013, 8:15 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> ---
> 
> Don't to extra job, when encoding is detected just return encoding.
> 
> 
> Diffs
> -
> 
>   generators/txt/document.h 08babea 
>   generators/txt/document.cpp badb647 
> 
> Diff: http://git.reviewboard.kde.org/r/109119/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Azat Khuzhin
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 109119: Txt backend: Drop detectEncoding method and just feed more data to KEncodingProber

[Commit Hook]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109119/#review28300
---


This review has been submitted with commit 
b5abd493ab80f642388b1ae055e7f06cfa1c59d5 by Albert Astals Cid on behalf of Azat 
Khuzhin to branch master.

- Commit Hook


On Feb. 25, 2013, 8:15 p.m., Azat Khuzhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109119/
> ---
> 
> (Updated Feb. 25, 2013, 8:15 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> ---
> 
> Don't to extra job, when encoding is detected just return encoding.
> 
> 
> Diffs
> -
> 
>   generators/txt/document.h 08babea 
>   generators/txt/document.cpp badb647 
> 
> Diff: http://git.reviewboard.kde.org/r/109119/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Azat Khuzhin
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 109119: Txt backend: Drop detectEncoding method and just feed more data to KEncodingProber

[Azat Khuzhin]

> On Feb. 27, 2013, 10:43 p.m., Albert Astals Cid wrote:
> > Code looks, ok, but now the big question, is this really an improvement? I 
> > guess the only improvement would be speed-wise and not 
> > encoding-detection-wise. Have you opened really big files and noticed it is 
> > faster? Otherwise maybe there is no really need to change the existing code 
> > if it works as well?

Yes it is only speed-wise improvement, as I already told.
> Description:
> Don't to extra job, when encoding is detected just return encoding.

For big files there is improvement, but this files is quite big (~900 000  
lines long), if we oriented on books
Just two times faster for such number of lines.

Maybe there is no need in this patch, but for me it more appropriate way.


- Azat


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109119/#review28231
---


On Feb. 25, 2013, 8:15 p.m., Azat Khuzhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109119/
> ---
> 
> (Updated Feb. 25, 2013, 8:15 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> ---
> 
> Don't to extra job, when encoding is detected just return encoding.
> 
> 
> Diffs
> -
> 
>   generators/txt/document.h 08babea 
>   generators/txt/document.cpp badb647 
> 
> Diff: http://git.reviewboard.kde.org/r/109119/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Azat Khuzhin
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 109119: Txt backend: Drop detectEncoding method and just feed more data to KEncodingProber

[Albert Astals Cid]

> On Feb. 27, 2013, 10:43 p.m., Albert Astals Cid wrote:
> > Code looks, ok, but now the big question, is this really an improvement? I 
> > guess the only improvement would be speed-wise and not 
> > encoding-detection-wise. Have you opened really big files and noticed it is 
> > faster? Otherwise maybe there is no really need to change the existing code 
> > if it works as well?
> 
> Azat Khuzhin wrote:
> Yes it is only speed-wise improvement, as I already told.
> > Description:
> > Don't to extra job, when encoding is detected just return encoding.
> 
> For big files there is improvement, but this files is quite big (~900 000 
>  lines long), if we oriented on books
> Just two times faster for such number of lines.
> 
> Maybe there is no need in this patch, but for me it more appropriate way.

Ok, i'm going to commit it. I have created a file with 1.7 million lines and 
the old code takes 270ms in running detectEncoding over that file and the new 
one takes 0ms, but still there is something out there taking a mch 
longer time so it's optimizing the wrong part of the code imo.


- Albert


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109119/#review28231
---


On Feb. 25, 2013, 8:15 p.m., Azat Khuzhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109119/
> ---
> 
> (Updated Feb. 25, 2013, 8:15 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> ---
> 
> Don't to extra job, when encoding is detected just return encoding.
> 
> 
> Diffs
> -
> 
>   generators/txt/document.h 08babea 
>   generators/txt/document.cpp badb647 
> 
> Diff: http://git.reviewboard.kde.org/r/109119/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Azat Khuzhin
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 109119: Txt backend: Drop detectEncoding method and just feed more data to KEncodingProber

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109119/#review28231
---


Code looks, ok, but now the big question, is this really an improvement? I 
guess the only improvement would be speed-wise and not encoding-detection-wise. 
Have you opened really big files and noticed it is faster? Otherwise maybe 
there is no really need to change the existing code if it works as well?

- Albert Astals Cid


On Feb. 25, 2013, 8:15 p.m., Azat Khuzhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109119/
> ---
> 
> (Updated Feb. 25, 2013, 8:15 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> ---
> 
> Don't to extra job, when encoding is detected just return encoding.
> 
> 
> Diffs
> -
> 
>   generators/txt/document.h 08babea 
>   generators/txt/document.cpp badb647 
> 
> Diff: http://git.reviewboard.kde.org/r/109119/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Azat Khuzhin
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 109119: Txt backend: Drop detectEncoding method and just feed more data to KEncodingProber

[Azat Khuzhin]

> On Feb. 23, 2013, 6:51 p.m., Azat Khuzhin wrote:
> > generators/txt/document.cpp, line 69
> > 
> >
> > Maybe we must use `array.left( i )` here?
> > And increment i += chunkSize before.
> 
> Albert Astals Cid wrote:
> Seems to me you should actually kill the detectEncoding function so you 
> use a single encoding detector all the time and just feed() it more data

Thanks! Fixed.


- Azat


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109119/#review27938
---


On Feb. 25, 2013, 8:15 p.m., Azat Khuzhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109119/
> ---
> 
> (Updated Feb. 25, 2013, 8:15 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> ---
> 
> Don't to extra job, when encoding is detected just return encoding.
> 
> 
> Diffs
> -
> 
>   generators/txt/document.h 08babea 
>   generators/txt/document.cpp badb647 
> 
> Diff: http://git.reviewboard.kde.org/r/109119/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Azat Khuzhin
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 109119: Txt backend: Drop detectEncoding method and just feed more data to KEncodingProber

[Azat Khuzhin]

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

(Updated Feb. 25, 2013, 8:15 p.m.)


Review request for Okular.


Changes
---

Drop detectEncoding method and just feed more data to KEncodingProber


Summary (updated)
-

Txt backend: Drop detectEncoding method and just feed more data to 
KEncodingProber


Description
---

Don't to extra job, when encoding is detected just return encoding.


Diffs (updated)
-

  generators/txt/document.h 08babea 
  generators/txt/document.cpp badb647 

Diff: http://git.reviewboard.kde.org/r/109119/diff/


Testing
---


Thanks,

Azat Khuzhin

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel