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

Improve Exceptions support #65

Merged
merged 35 commits into from Jan 14, 2021
Merged

Conversation

quasilyte
Copy link
Contributor

@quasilyte quasilyte commented Dec 8, 2020

Compiler memory consumption comparison:

old:

  memory.rss: 13711200256
  memory.rss_peak: 13970407424

  compilation.transpilation_time: 47.7062
  compilation.object_out_size: 39337976

new (with exceptions tracking + @kphp-throws vector inside func info):

  memory.rss: 13838069760 (+1%)
  memory.rss_peak: 14024077312 (+0.6%)

  compilation.transpilation_time: 46.0325
  compilation.object_out_size: 40028272

Notes:

  • Added support for multiple catch clauses per try block
  • Added \Throwable interface; \Exception now implements \Throwable
  • Added \Error class that implements \Throwable + SPL Exception classes
  • Old KPHP code will generate the same C++ code for the try/catch blocks
  • All code generated code diff is new Exception(...) => __exception_set_location(new Exception(...), __FILE__, __LINE__) and an instance cast of exception var if it's not Throwable
  • Code that catches all possible exceptions does not propagate any (exceptions are tracked)
  • This PR does not implement finally on purpose

--

Enabled exceptions inheritance.

  • updated CFG and other passes so they handle the program flow correctly

  • codegen generates 2 new cases for the try/catch ops.
    If exception class has derived classes, is_a() is used;
    If exception class has no derived classes, hash comparison is used;
    When the code catches \Exception class, same codegen is used.

Exceptions in CFG propagate like this: if try block catches
all exceptions, we consider that nothing propagates.
Otherwise, we consider all exceptions that can be created inside try block do propagate.
We may do exceptions tracking here as well, but it doesn't affect
the code generation right now.

--

Added PreprocessException pass.

Add __FILE__ and __LINE__ arguments to the Exception-derived
construction call in the PreprocessException pass instead of
doing it in the lexer.

The problem with lexer is that it tries to know about the
Exception class and adds these arguments too early.

If Exception class can be extended, then we don't know
whether a new T() expression constructs an exception class.
The PreprocessException has class data info, so it can
perform this operation with more accuracy.

We also change the Exception class constructor signature so
it's compatible with the PHP version. It doesn't accept the
file and line arguments now; to set these fields, a new builtin function
is added that is inserted by the compiler.

This transformation can be described as:

    new T(args...)
    =>
    __exception_set_location(new T(args...), __FILE__, __LINE__)

Where __exception_set_location() call returns the modified exception.

This patch does not add missing Exception-related features,
but it makes it's needed to make the work in that direction possible.

@quasilyte quasilyte changed the title Improve Exceptions support WIP: Improve Exceptions support Dec 9, 2020
@unserialize unserialize added the enhancement New feature or request label Dec 9, 2020
@quasilyte quasilyte force-pushed the quasilyte/improve_exceptions_support branch 2 times, most recently from d8d748e to 8cdd8d7 Compare December 10, 2020 09:39
@quasilyte quasilyte changed the title WIP: Improve Exceptions support Improve Exceptions support Dec 10, 2020
@quasilyte
Copy link
Contributor Author

The code may be rough in some places, but it's ready for the review.
I tested the compiler on vkcom, the diff is predictable (__exception_set_location).

@quasilyte
Copy link
Contributor Author

php-src tests are failing due to the fact that KPHP now resolves the classes used in catch.
Before that, it was ignoring them and implied \Exception.

Right now we're missing TypeError exception class (and maybe some other classes, I need to reproduce this thing locally).

In stage = [Inject locations into the created exceptions]:
  [file = php-src/ext/standard/tests/strings/strrpos_offset.phpt]
  [function = src_strrpos_offset_phpt56dbca052236ad80:9]
  [line = 9]
//   9:    echo $e->getMessage(), "\n ";
Can't find class: TypeError

@quasilyte quasilyte changed the title Improve Exceptions support WIP: Improve Exceptions support Dec 16, 2020
@quasilyte quasilyte force-pushed the quasilyte/improve_exceptions_support branch from d73c6e4 to bc6a194 Compare December 18, 2020 10:35
@quasilyte quasilyte changed the title WIP: Improve Exceptions support Improve Exceptions support Dec 18, 2020
@quasilyte
Copy link
Contributor Author

Some extra generated code diff is expected now: since CurException is a Throwable now, not Exception, when there is a catch on \Exception we need to do an instance_cast there.

@quasilyte quasilyte force-pushed the quasilyte/improve_exceptions_support branch 2 times, most recently from a7b3614 to df4e06f Compare December 18, 2020 19:35
@quasilyte quasilyte changed the title Improve Exceptions support WIP: Improve Exceptions support Dec 21, 2020
@quasilyte quasilyte force-pushed the quasilyte/improve_exceptions_support branch 4 times, most recently from 0c10c1f to 5d49e6a Compare December 23, 2020 14:58
@quasilyte
Copy link
Contributor Author

quasilyte commented Dec 23, 2020

Tested exception tracking with KPHP_VERBOSITY=2.
Same functions.total_throwing value for vkcom.

Also compared the passes execution time.
The most affected passes are CalcActualCallsEdgesPass and FilterOnlyActuallyUsedFunctionsF. Their performance is comparable for our current code. It will be slower for the code that throws different kinds of exceptions.

@quasilyte quasilyte force-pushed the quasilyte/improve_exceptions_support branch from 5d49e6a to 691c275 Compare December 23, 2020 17:44
@quasilyte quasilyte changed the title WIP: Improve Exceptions support Improve Exceptions support Dec 24, 2020
@quasilyte
Copy link
Contributor Author

Ready for review.

@quasilyte quasilyte force-pushed the quasilyte/improve_exceptions_support branch from ac70886 to fcc74c0 Compare December 24, 2020 11:03
runtime/exception.h Outdated Show resolved Hide resolved
runtime/exception.h Outdated Show resolved Hide resolved
functions.txt Show resolved Hide resolved
compiler/data/function-data.h Outdated Show resolved Hide resolved
compiler/pipes/final-check.cpp Outdated Show resolved Hide resolved
@quasilyte quasilyte force-pushed the quasilyte/improve_exceptions_support branch 2 times, most recently from 2b4100b to a7bdd88 Compare December 30, 2020 12:00
@unserialize unserialize added this to the next milestone Jan 13, 2021
docs/kphp-language/howto-by-kphp/exceptions.md Outdated Show resolved Hide resolved
compiler/code-gen/declarations.cpp Outdated Show resolved Hide resolved
compiler/data/function-data.h Outdated Show resolved Hide resolved
compiler/pipes/calc-actual-edges.h Outdated Show resolved Hide resolved
Add __FILE__ and __LINE__ arguments to the Exception-derived
construction call in the PreprocessException pass instead of
doing it in the lexer.

The problem with lexer is that it tries to know about the
Exception class and adds these arguments too early.

If Exception class can be extended, then we don't know
whether a `new T()` expression constructs an exception class.
The PreprocessException has class data info, so it can
perform this operation with more accuracy.

We also change the Exception class constructor signature so
it's compatible with the PHP version. It doesn't accept the
file and line arguments now; to set these fields, a new builtin function
is added that is inserted by the compiler.

This transformation can be described as:

        new T(args...)
        =>
        __exception_set_location(new T(args...), __FILE__, __LINE__)

Where __exception_set_location() call returns the modified exception.

This patch does not add missing Exception-related features,
but it makes it's needed to make the work in that direction possible.
Connect the graph like this:

	e1                        catch1
	   \                    /
	e2 -- catch_list_start -- catch2
	   /                    \
	eN                        catchN

Instead of this:

	      catch1
	    /
	e1 -- catch2
	    \
	      catchN
              catch1
            /
        e2 -- catch2
            \
              catchN
              catch1
            /
        eN -- catch2
            \
              catchN
CurException is now Throwable as it can be either Exception or Error.
- use unordered sets for kphp-test-throws
- use dedicated op instead of __set_exception_location()
@quasilyte quasilyte force-pushed the quasilyte/improve_exceptions_support branch from 216753d to 179b79c Compare January 13, 2021 17:35
@quasilyte quasilyte force-pushed the quasilyte/improve_exceptions_support branch from 68164d5 to b7edab4 Compare January 14, 2021 11:54
@quasilyte quasilyte merged commit 9d9a811 into master Jan 14, 2021
@quasilyte quasilyte deleted the quasilyte/improve_exceptions_support branch January 14, 2021 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants