w3c / longtasks

Long Task API
https://w3c.github.io/longtasks/
Other
247 stars 32 forks source link

containerSrc could be long (a data URI) #64

Open tdresser opened 5 years ago

tdresser commented 5 years ago

We should likely truncate containerSrc in cases where it's too long.

domenic commented 5 years ago

We don't do this for anything else on the platform that exposes a URL... what's the rationale?

tdresser commented 5 years ago

If you completely delete an element, it's associated PerformanceEntry could keep the full data URI alive for no good reason. I believe this is different from other ways URLs are exposed today.

This may just be premature optimization though.

@yoavweiss @rniwa @toddreifsteck, any thoughts?

toddreifsteck commented 5 years ago

My thought is that this is premature optimization. @rniwa ?

hawkinsw commented 5 years ago

If you completely delete an element, it's associated PerformanceEntry could keep the full data URI alive for no good reason. I believe this is different from other ways URLs are exposed today.

Can you explain to me why the PerformanceEntry would stay alive for an element that is completely deleted? I am sorry to be stupid.

This may just be premature optimization though.

@yoavweiss @rniwa @toddreifsteck, any thoughts?

rniwa commented 5 years ago

I mean... there is no mechanism for PerformanceEntry to removed for long tasks right now, right?

hawkinsw commented 5 years ago

At this point then, what we are saying is that an implementer might choose to keep it around because there is nothing in the spec that mandates its deletion? In such a case, a long URL could have a negative impact on memory consumption of the program?

On Mon, Jul 1, 2019, 1:02 AM Ryosuke Niwa notifications@github.com wrote:

I mean... there is no mechanism for PerformanceEntry to removed for long tasks right now, right?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/w3c/longtasks/issues/64?email_source=notifications&email_token=ACCP2CRFSVF2GSCFXCVC7S3P5GFURA5CNFSM4HXSAP62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY47XYA#issuecomment-507116512, or mute the thread https://github.com/notifications/unsubscribe-auth/ACCP2CVHOJOTMQHDMLZ2FGTP5GFURANCNFSM4HXSAP6Q .

toddreifsteck commented 5 years ago

The PerformanceEntry is enqueued to Performanee Timeline. Is the underlying question whether the Performance Timeline can delete the entry if there is no buffering behavior specified?

rniwa commented 5 years ago

At this point then, what we are saying is that an implementer might choose to keep it around because there is nothing in the spec that mandates its deletion

It's not so much that an implementation may choose to keep it in the timeline. The specifications mandates that whatever entry added to the timeline remain in the timeline unless the buffering limit is reached, or it's explicitly cleared. Whether the associated element is removed from the document is irrelevant in this regard.

hawkinsw commented 5 years ago

I think that's what I was asking, Todd! Thanks for being accurate.

I think that Ryosuke answered my question.

I do wonder, though, why we did not address removing Performance Entries from the Performance Timeline if/when their associated elements are deleted from the document.

On Mon, Jul 1, 2019 at 4:26 PM Todd Reifsteck notifications@github.com wrote:

The PerformanceEntry is enqueued to Performanee Timeline. Is the underlying question whether the Performance Timeline can delete the entry if there is no buffering behavior specified?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/w3c/longtasks/issues/64?email_source=notifications&email_token=ACCP2CRT3AZMY3LY5S7UITTP5JR7NA5CNFSM4HXSAP62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY7IAOA#issuecomment-507412536, or mute the thread https://github.com/notifications/unsubscribe-auth/ACCP2CTCSIHELD7ULNXHPQ3P5JR7NANCNFSM4HXSAP6Q .

yoavweiss commented 5 years ago

The PerformanceEntry is enqueued to Performance Timeline. Is the underlying question whether the Performance Timeline can delete the entry if there is no buffering behavior specified?

IIRC, currently for LongTasks, they are only queued for a PerformanceObserver, which means that a site that ignores those entries will not keep that URL alive. We are talking about keeping them in the timeline in the future though (through explicit registration).

It may make sense to explicitly set apart data URIs from e.g. HTTP schemes.

It's also worth noting that at least in some implementations, those URLs can take up that space exactly once, even if multiple LongTask entries point at them.

I do wonder, though, why we did not address removing Performance Entries from the Performance Timeline if/when their associated elements are deleted from the document.

IMO, it doesn't really make sense to remove such entries from the timeline, as they did happen (and impacted the user). Later removal of the element doesn't change that.

npm1 commented 5 years ago

Yes, right now there is no buffered flag behavior nor performance timeline buffering for longtask entries, so they would only need to be kept alive if a PerformanceObserver looking at them stores them in JS. Is it common to have long inline scripts? Exposing the full URI does not seem to be flagged to Chrome as a memory issue thus far. Maybe it could become one if we support buffering longtask entries in the future.

toddreifsteck commented 5 years ago

Per call on 8/1: Consider an arbitrary limit for data uri's universally for all Performance Timeline exposed PerformanceEntry's.

yoavweiss commented 2 years ago

@npm1 - I know that we're applying limits on URL length elsewhere. Would be good to consolidate our approach.

npm1 commented 2 years ago

Yes, although for different resource types. Consolidating still seems reasonable to me.

npm1 commented 2 years ago

Similar issue: https://github.com/WICG/element-timing/issues/64

noamr commented 1 year ago

I wonder if we should follow this guideline, and leave this limitation implementation-defined.