Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 253690

Summary: [scalability] Parser needs a lot of memory parsing large initializer expressions
Product: [Tools] CDT Reporter: Anton Leherbauer <aleherb+eclipse>
Component: cdt-parserAssignee: Markus Schorn <mschorn.eclipse>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: cdtdoug, eclipse.sprigogin, filip.gustafsson, mikekucera, wbprio
Version: 5.0Flags: cdtdoug: iplog-
Target Milestone: 5.0.2   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Sample file
none
proposed fix
none
the fix
none
the fix
none
follow up on the fix
none
further optimization for CDT 6.0
none
follow up: fixes detection of names in designators none

Description Anton Leherbauer CLA 2008-11-04 08:40:32 EST
Parsing the following sample

char large_array[] = {
 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 [repeat 100000 times]
};

requires more than 256M of heap space.
Comment 1 Anton Leherbauer CLA 2008-11-04 08:48:51 EST
Created attachment 116932 [details]
Sample file
Comment 2 Markus Schorn CLA 2008-11-05 06:42:40 EST
The problem is two-fold:
* the parser keeps a reference to the first token in order to backtrack, if the
  initializer cannot be parsed, therefore all the tokens need to be held in 
  memory at once.
* The ast contains a lot of literal expressions. The indexer does not need to
  look at these expressions, we need an option for the ast creation that allows
  skipping trivial expressions in compound-initializers. 
Comment 3 Markus Schorn CLA 2008-11-06 05:09:35 EST
Created attachment 117176 [details]
proposed fix

The fix consists of multiple changes:
(1) parsers no longer hold on to tokens when parsing aggregate initializers
(2) an additional option to skip literal expressions in aggregate initializers,
    option is used by ASTProvider, CModelBuilder2 and indexer.
(3) changed ASTProvider such that it does not attempt to provided ast for 
    closed translation units, such that we don't generate AST in scalability
    mode. New name for flag: WAIT_YES -> WAIT_IF_OPEN
(4) Reviewed and reduced the usage of WAIT_IF_OPEN flag, such that attempts to
    generate AST for non-active editors are minimized.

Especially for the changes related to (3) and (4) it would be good to have
my changes reviewed:
2:   CModelBuilder2, ASTCache
3:   ASTProvider
4:   CElementHyperlinkDetector, InactiveCodeHighliting,
     SemanticHighlightingReconciler, CorrectionCommandHandler, 
     QuickAssistLightBulbUpdater, LinkedNamesAssistProposal
Comment 4 Anton Leherbauer CLA 2008-11-06 05:54:13 EST
Some UI tests fail because the AST is requested when the working copy is not yet open, e.g. the HyperlinkTests.
A simple fix for the tests is to make sure the reconciler has run:
EditorTestHelper.joinReconciler(EditorTestHelper.getSourceViewer(editor), 100, 500, 10);
Otherwise the changes look good.
Comment 5 Markus Schorn CLA 2008-11-06 08:34:48 EST
Created attachment 117193 [details]
the fix

Thanks Toni, I have updated the testcases to wait for the reconciler.
Comment 6 Markus Schorn CLA 2008-11-06 08:49:55 EST
Created attachment 117194 [details]
the fix

patch with minor cleanup work, such as updated copy right headers
Comment 7 Markus Schorn CLA 2008-11-06 08:59:56 EST
Created AST2Tests.testScalabilityOfLargeTrivialInitializer_Bug252970(),

Fixed in 5.0.2 > 20081106.
Comment 8 Markus Schorn CLA 2008-11-07 03:56:01 EST
Created attachment 117293 [details]
follow up on the fix

I must avoid using a visitor on the ast before ambiguity resolution.
Comment 9 Markus Schorn CLA 2008-11-07 04:55:20 EST
Created attachment 117297 [details]
further optimization for CDT 6.0

I improved interface + implementation of IASTLiteralExpression to avoid creation of superfluous string-objects. This patch cannot be applied to 5.0.x
Comment 10 Markus Schorn CLA 2008-11-10 10:11:06 EST
Created attachment 117439 [details]
follow up: fixes detection of names in designators
Comment 11 Markus Schorn CLA 2008-12-17 09:00:07 EST
*** Bug 258944 has been marked as a duplicate of this bug. ***
Comment 12 Mike Kucera CLA 2009-01-02 09:59:23 EST
(In reply to comment #7)
> Created AST2Tests.testScalabilityOfLargeTrivialInitializer_Bug252970(),

That's the wrong bug number. Fixed in Head.
Comment 13 Sergey Prigogin CLA 2009-01-10 18:53:12 EST
testScalabilityOfLargeTrivialInitializer_Bug253690 consistently fails in HEAD. On Windows with JRE 1.6.0-11 it gives 3254464 expected < 3120512, on Linux with JRE 1.6.0-1 - 3120552 expected < 3120512
Comment 14 Markus Schorn CLA 2009-01-13 08:48:24 EST
(In reply to comment #13)
> testScalabilityOfLargeTrivialInitializer_Bug253690 consistently fails in HEAD.
> On Windows with JRE 1.6.0-11 it gives 3254464 expected < 3120512, on Linux with
> JRE 1.6.0-1 - 3120552 expected < 3120512

Thanks, I use a weaker assertion now.
Comment 15 Filip Gustafsson CLA 2009-11-18 02:51:18 EST
Sorry for the delay but I have now tested this in CDT 6.0.0 and it works fine.