Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Ignore specific methods / Add Call-Log Threshold #51

Open
SeriousM opened this issue Feb 24, 2014 · 12 comments
Open

Feature Request: Ignore specific methods / Add Call-Log Threshold #51

SeriousM opened this issue Feb 24, 2014 · 12 comments

Comments

@SeriousM
Copy link

When I create a game library, I usually have a game-loop function.
Because Theseus logs every object for each call, my RAM runs low very quickly in these cases.

image

As you can see, the RAM increases to 600MB and was still growing. Luckily I have 16GB RAM.
Usually Brackets uses 172-200MB.

Could we get a possibility to ignore certain methods? Maybe via a comment or something.
Or maybe Theseus could get a threshold to hold only the last 20/50/100 calls (but still increase the counter)?

Thanks

@alltom
Copy link
Member

alltom commented Feb 24, 2014

Sure thing! It's a problem I predicted, but nobody ever reported, so I thought perhaps it wasn't very common. :)

fondue will not instrument a file containing /*theseus instrument: false */ (so you can use that right away if your game loop is pretty much in a file by itself). I think what fondue should also do is look for /*theseus capture: false */ or something inside of a function to indicate that its arguments and return value should not be stored, but that its call count should still be incremented.

It's slightly tricky because the arguments are captured at the call site as well as inside that function (in case the function isn't instrumented, like console.log). I don't think that will be too much of a problem, though.

@SeriousM
Copy link
Author

I think what fondue should also do is look for /*theseus capture: false */ or something inside of a function to indicate that its arguments and return value should not be stored, but that its call count should still be incremented.

I tried that but it didn't work, even with difference space-styles (start, end).

I don't think that will be too much of a problem, though.

Does that mean you're going to implement that change?

Cheers

@alltom
Copy link
Member

alltom commented Feb 25, 2014

Yeah, sorry I was unclear. I'm proposing to make /*theseus capture: false */ work as I described, in a future release. /*theseus instrument: false */ already works for whole files.

@SeriousM
Copy link
Author

Alright, that would be a great help!

What about the recording limit, is that too much work? Maybe I can help if
you point me to the right files.

@alltom
Copy link
Member

alltom commented Feb 25, 2014

That'd be great!

General project documentation for working on Theseus is here: https://github.com/adobe-research/theseus/wiki/Theseus-Development

You'd be working in tracer.js in fondue. Probably what you would do is modify pushNewInvocation to construct the Invocation object without value's for the function's arguments if invocationsByNodeId[] for that nodeId already has over a certain number of calls. There are going to be lots of little things to do for usability. Off the top of my head:

@SeriousM
Copy link
Author

Hi @alltom, thanks for the brainstorming!
I think we shouldn't limit the number of total records rather than having something like a rolling log. Therefore we don't have to modify fondue that much and brackets (theseus ui?) just discards everything that exceeds the limit (FIFO).
What do you think about that? Is my guess correct that this wouldn't be that much work?

@alltom
Copy link
Member

alltom commented Feb 26, 2014

Oh, I think I misunderstood your original problem then. I had assumed that if Brackets was using too much RAM, fondue (which runs in Chrome on the debugged web page) was probably also using too much RAM, since the information in Brackets is always a subset of what's in fondue.

However, if the problem is actually that the DOM for the log panel is growing too large, that's different! The fix you just proposed makes sense then.

The functions you'll need to modify are appendLogs and render. I don't think it would take much for you to prototype something. To me, it's unclear what should happen to log entries arrive that are children of log entries that have been flushed from the history, but once you have something going that answer may become clear. Maybe it makes sense to empty the DOM nodes but keep them around, and if new children come in, put placeholder data into them, so it looks like:

  • [entry removed]
    • [entry removed]
      • log entry
  • log entry
    • log entry
  • ...

But I guess you'll see! :)

@SeriousM
Copy link
Author

Sounds good, I'll try to get something together!

@SeriousM
Copy link
Author

Hey, I really tried to get this to work but I couldn't.
Maybe one of your team has time for that?

@alltom
Copy link
Member

alltom commented Mar 20, 2014

It's just me, but I'll see what I can do. I don't have terribly much time for Theseus these days, though. :(

@SeriousM
Copy link
Author

I thought it's under the umbrella from adobe, isn't it?

@alltom
Copy link
Member

alltom commented Mar 20, 2014

Yes, but I'm the team at the moment, though the Brackets team also sends some excellent pull requests. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants