Closed GoogleCodeExporter closed 9 years ago
This is due to recent changes effecting the system- I noticed this last night.
For
some reason we're validating after each card is added, which means a
logarithmic time
problem. Before, say, r1940, I'd been testing with Maxglee- 34k cards, no major
slowdown. I've got a little time- not much, but some- so I'll look at this
tonight.
Original comment by wagic.jeck@gmail.com
on 2 Mar 2010 at 1:45
sorry, crossed wires- exponential, not logarithmic.
Original comment by wagic.jeck@gmail.com
on 2 Mar 2010 at 1:47
Ok. I increased the issue by removing a "if (!filterRoot) return;" in the
validate()
function.
Removing that line was necessary for the validate() to actually work with Deck
stats
computation.
But now I understand a few things.
When a card is clicked, stw.needUpdate is set to 1, which is correct.
However, we should perform the stats update (and therefore, the "validate")
only
when they are actually displayed.
If you agree that removing "if (!filterRoot) return;" was necessary, and that
it
just "reveals" the issue but is not its cause, then I think I can work on the
issue
in a few hours too.
Original comment by wagic.the.homebrew@gmail.com
on 2 Mar 2010 at 1:56
Not sure if it's the same issue, but I experience much longer deck loading
times since
r1948. Result of a quick test (loading a deck with 8 cards in it while having a
huge
collection):
in r1947: 2 seconds
in r1948: 25 seconds
Tested on Windows XP on an Athlon64 3500+.
Original comment by Psyyri...@gmail.com
on 2 Mar 2010 at 2:13
So, yes, the question is:
Why do we need to call "validate()" everytime a card is added or inserted ?
I'm guessing that's the lazy way of adding the card to the "validated" array.
I see there are sorting issues as well, which is probably the real reason you
call
validate after each time you add a card...
Suggestion to improve this:
- "validate" should not be an array, but a map, sorted the same way we sort the
cards map. Therefore
-- When adding a card, we check if it matches the filter, and if so, we add it
to
validated (automatically sorted)
-- when removing a card, we look for it in validated (find) and remove it if it
exists. Done
See
http://code.google.com/p/wagic/source/browse/trunk/projects/mtg/src/WDataSrc.cpp
#428
and
http://code.google.com/p/wagic/source/browse/trunk/projects/mtg/src/WDataSrc.cpp
#447
Original comment by wagic.the.homebrew@gmail.com
on 2 Mar 2010 at 2:19
Psyringe: yes, that's the issue. I removed a check from the function
"validate()". I
still believe removing it was the correct thing to do. But this function is
extremely costy, and calling it all the time is not efficient.
I believe the thing I removed (if(!filterRoot) return;) was a dangerous patch
that
was hiding the issue in most cases (a.k.a when no filter was in place).
Jeck I need your confirmation on this.
Original comment by wagic.the.homebrew@gmail.com
on 2 Mar 2010 at 2:22
Psy, that's definitely related. We're now caching the result of filtering (what
the
"validated" vector is for, so we don't run filters every frame) even when no
filters
are being applied.
Wololo, I'm not certain removing that line is merited. The interface isn't very
clean
right now, but there are ways for the stats thing to get filtered/unfiltered
stats...
it's just that in some places they were being overlooked. Honestly, the code for
various "fixes" to statistics has been pretty rushed, I've not had a decent
chunk of
time to work on wagic in awhile now... but I've been working on the code since I
posted... I've probably got another hour (at most two) before I have to get
back to
other responsibilities, let me work it over during that time and we'll see what
happens. I think I can have it a lot cleaner, at least, and if it's not fixed
by then
it'll at least be easier to read :)
Original comment by wagic.jeck@gmail.com
on 2 Mar 2010 at 2:23
Ok. just review my change 1948. It's 5 lines. You'll see what my mistake is,
but
you'll also understand what I'm trying to achieve with this.
Basically the deck is not properly reinitialized to "no filter" when computing
the
stats. That's what I was trying to do here.
Original comment by wagic.the.homebrew@gmail.com
on 2 Mar 2010 at 2:29
Oh, and I stil think putting back the "if !filterRoot" in the validate function
is
dangerous. The function should validate EVEN if there is no filter
Original comment by wagic.the.homebrew@gmail.com
on 2 Mar 2010 at 2:30
... I'm not certain I understand what you mean. If there is no condition to be
checked, why iterate through the set, making an exact copy of vector<MTGCard*>
cards?
I'll see if you're available on google talk.
Original comment by wagic.jeck@gmail.com
on 2 Mar 2010 at 3:08
psyringe, Jeck potentially fixed that, can you verify ?
Original comment by wagic.the.homebrew@gmail.com
on 2 Mar 2010 at 10:29
i see it is already fixed on my psp and PC
Original comment by zoleg...@gmail.com
on 3 Mar 2010 at 1:14
marked as fixed, please reopen if the issue happens again
Original comment by wagic.the.homebrew@gmail.com
on 4 Mar 2010 at 11:32
Original issue reported on code.google.com by
wagic.the.homebrew@gmail.com
on 2 Mar 2010 at 12:54