lentschi / ngx-ellipsis

Multiline text with ellipsis for angular 9+
MIT License
96 stars 22 forks source link

Performance issue #21

Closed UlisesCeca closed 5 years ago

UlisesCeca commented 5 years ago

Hello,

I am using this library, which is great so far, on my Angular project.

However, I detected that when I add use this library, the page where I use this component becomes really slow.

Here you can see better in a video how long it takes to load some cards after the spinner dissapears but if I don't use this library, they load inmediatly.

Is there anything that can be done about this?

https://youtu.be/J_Vl7D10uWY

Thanks.

lentschi commented 5 years ago

Hi @UlisesCeca!

Short answer: For three ellipsis elements per page (all that I can see in your video) the performance should not be a real problem, no! (If it is for you, please create a StackBlitz or likewise so I can reproduce the problem).

But yeah that said, I have to admit that the library does cause quite some CPU load if you have many ellipsis elements. (I could not find a very efficient way to calculate the ellipsis: What my library does is simply take a guess at how many characters will fit into the container, increase the number of characters, if there is still space, or decrease it if it's causing overflow. It repeats that calculation until it has finally found the exact number of characters that will just fit. This does sound like madness, I know, but consulting other - angular-unrelated - libraries they seem to be doing the exact same thing - e.g.: https://stackoverflow.com/questions/536814/insert-ellipsis-into-html-tag-if-content-too-wide/9071361#answer-9071361 - Like in this sample I use binary tree search to optimize the "trial and error" part and reduce the madness.)

Possible solution: So if you have a long scrolling list with lots of ellipsis containers, performance will indeed be poor and you should consider using a virtual scroll solution such as cdk-virtual-scroll-viewport. Note however that there was an issue raised that cdk-virtual-scroll-viewport and ngx-ellipsis together cause glitches: #15. I provided a working and nicely performing StackBlitz sample (with 100000 ellipsis elements) on the issue page though, and nobody could provide one that fails so far - so maybe these glitches only happen in very specific cases.

UlisesCeca commented 5 years ago

I think what's causing it is that every time I load more items, the resizing listener that there is in your code is triggering. I'll try to remove it in a fork and let you know. If this were the problem it'd be nice to add that listener optionally through a variable.

lentschi commented 5 years ago

@UlisesCeca If the resize listener is causing the trouble, you can just add the ellipsis-resize-detection="window" attribute (or maybe even manual - for details see the README)

UlisesCeca commented 5 years ago

@lentschi that totally fixed it! Thank you very much!!

lentschi commented 5 years ago

@UlisesCeca Great! Funny though, that the default detection causes such bad performance...

ngx-ellipsis uses wnr/element-resize-detector by default. In the test cases I've had so far, performance was fine. (Maybe it's somehow related to your css)

Anyhow - in most cases it'll be sufficient to trigger detection upon window resize (as long as no other DOM elements dynamically expand caused by some JS logic and 'steal' the ellipsis' space) - so if that works for you, I'm glad. :smile:

UlisesCeca commented 5 years ago

It's weird, @lentschi the first time the component loads it doesn't get ellipsis, is this normal behaviour?

lentschi commented 5 years ago

@UlisesCeca No, definitely not. But without a sample I'm afraid can't help. I'm not even sure I understand what you mean - You mean the first time the component containing the ellipsis loads or the first time any ellipsis loads?

Here's a StackBlitz sample that loads instantly: https://stackblitz.com/edit/angular-1smu3f-gxhf5p?file=app%2Fellipsis-example.ts

UlisesCeca commented 5 years ago

Sorry @lentschi I'm kinda new to Angular and haven't used pages like that so it's kinda hard for me to move my code there. I've found the reason tho.

It was because I had my code as follows:

<ng-container *ngIf="ellipsis else noEllipsis2">
       <div class="description" ellipsis-resize-detection="window" ellipsis [ellipsis-content]="work.description"></div>
</ng-container>

<ng-template #noEllipsis2>
      <div class="description" [class.ellipsis-scroll]="!ellipsis">
            {{ work.description }}
      </div>
</ng-template>

Appareantly the ng-container was the culprit.

Thank you for your time and this lib :)

lentschi commented 5 years ago

@UlisesCeca You're welcome! :smile:

But I just updated the StackBlitz with your code (which looks fine) and that too works, so your issue must lie somewhere else: https://stackblitz.com/edit/angular-1smu3f-3w1hbb

lentschi commented 5 years ago

For future reference: Performance might be increased by using a different approach than the binary search mentioned above: By wrapping each character (or each valid sequence of characters as defined by ellipsis-word-boundaries) in its own DOM element. Thus the position of each character could be determined which in turn reveals where the text would start overflowing without the need to guess. This approach was used in https://github.com/jjenzz/jquery.ellipsis for example. First however, it remains to be verified that this approach indeed does perform better: For long texts wrapping each character in a <span> might come at the cost of slow rendering.

(The binary search algorithm was also used by https://github.com/STAR-ZERO/jquery-ellipsis)

cmbkla commented 1 year ago

does cause quite some CPU load if you have many ellipsis elements

I am currently dealing with this across a large app. We used it in a table element, and find that when we hit about 15 items there is a noticeable impact on poorly resourced client machines. This is probably compounded by the resizing/"responsive" table stuff, I think we have the browser competing with the ellipsis lib to figure out how wide a column is supposed to be.

Something I had thought of as a mitigation is the addition of a "context" (arbitrary string) to the lib, letting it know that it can re-use the character calculation for all ellipsis that share a context. That way the lib can figure out the character limit for the first row, then re-use that calculation for all other rows.

lentschi commented 1 year ago

Something I had thought of as a mitigation is the addition of a "context" (arbitrary string) to the lib, letting it know that it can re-use the character calculation for all ellipsis that share a context. That way the lib can figure out the character limit for the first row, then re-use that calculation for all other rows.

@cmbkla Hm... What's the use-case for which you are proposing this "re-use" option? As far as I see it, this could only work for cells that contain exactly the same text and have exactly the same styles and dimensions. Does that happen a lot in your app? Or am I misunderstanding something?

Also - just in case you overlooked the suggestions further to the top in this issue - you could mitigate the performance impact using one or both of the following: